From 4b136e8ab13111fb9a6e530a71d8265bcbef1d96 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 09:51:57 +0300 Subject: [PATCH 1/9] cli/sc: tune error messages for forced save/send Here we're either saving or sending a transaction (depending on `out`), but not both. Refs. #2664. --- cli/smartcontract/smart_contract.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index a61222ecb..f96935d10 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -702,14 +702,11 @@ func invokeWithArgs(ctx *cli.Context, acc *wallet.Account, wall *wallet.Wallet, return sender, cli.NewExitError(errText, 1) } - action := "save" - process := "Saving" + action := "send" + process := "Sending" if out != "" { - action += "and send" - process += "and sending" - } else { - action = "send" - process = "Sending" + action = "save" + process = "Saving" } if !ctx.Bool("force") { return sender, cli.NewExitError(errText+".\nUse --force flag to "+action+" the transaction anyway.", 1) From 58707c2b1ec2bf730933f36524cc142f1795261f Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 10:53:49 +0300 Subject: [PATCH 2/9] context: handle the case when we have more sigs than needed We can technically have more signatures in the file than we need and it's OK, this case should be handled. --- pkg/smartcontract/context/context.go | 11 ++++-- pkg/smartcontract/context/context_test.go | 46 +++++++++++++++-------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/pkg/smartcontract/context/context.go b/pkg/smartcontract/context/context.go index fd768604d..cd4d09d0e 100644 --- a/pkg/smartcontract/context/context.go +++ b/pkg/smartcontract/context/context.go @@ -96,14 +96,19 @@ func (c *ParameterContext) AddSignature(h util.Uint160, ctr *wallet.Contract, pu return errors.New("public key is not present in script") } item.AddSignature(pub, sig) - if len(item.Signatures) == len(ctr.Parameters) { + if len(item.Signatures) >= len(ctr.Parameters) { indexMap := map[string]int{} for i := range pubs { indexMap[hex.EncodeToString(pubs[i])] = i } - sigs := make([]sigWithIndex, 0, len(item.Signatures)) + sigs := make([]sigWithIndex, len(item.Parameters)) + var i int for pub, sig := range item.Signatures { - sigs = append(sigs, sigWithIndex{index: indexMap[pub], sig: sig}) + sigs[i] = sigWithIndex{index: indexMap[pub], sig: sig} + i++ + if i == len(sigs) { + break + } } sort.Slice(sigs, func(i, j int) bool { return sigs[i].index < sigs[j].index diff --git a/pkg/smartcontract/context/context_test.go b/pkg/smartcontract/context/context_test.go index 0b73c5718..c873b63e8 100644 --- a/pkg/smartcontract/context/context_test.go +++ b/pkg/smartcontract/context/context_test.go @@ -96,24 +96,38 @@ func TestParameterContext_AddSignatureMultisig(t *testing.T) { sig := priv.SignHashable(uint32(c.Network), tx) require.Error(t, c.AddSignature(ctr.ScriptHash(), ctr, priv.PublicKey(), sig)) - indices := []int{2, 3, 0} // random order - for _, i := range indices { - sig := privs[i].SignHashable(uint32(c.Network), tx) - require.NoError(t, c.AddSignature(ctr.ScriptHash(), ctr, pubs[i], sig)) - require.Error(t, c.AddSignature(ctr.ScriptHash(), ctr, pubs[i], sig)) + indices := []int{2, 3, 0, 1} // random order + testSigWit := func(t *testing.T, num int) { + for _, i := range indices[:num] { + sig := privs[i].SignHashable(uint32(c.Network), tx) + require.NoError(t, c.AddSignature(ctr.ScriptHash(), ctr, pubs[i], sig)) + require.Error(t, c.AddSignature(ctr.ScriptHash(), ctr, pubs[i], sig)) - item := c.Items[ctr.ScriptHash()] - require.NotNil(t, item) - require.Equal(t, sig, item.GetSignature(pubs[i])) + item := c.Items[ctr.ScriptHash()] + require.NotNil(t, item) + require.Equal(t, sig, item.GetSignature(pubs[i])) + } + + t.Run("GetWitness", func(t *testing.T) { + w, err := c.GetWitness(ctr.ScriptHash()) + require.NoError(t, err) + v := newTestVM(w, tx) + require.NoError(t, v.Run()) + require.Equal(t, 1, v.Estack().Len()) + require.Equal(t, true, v.Estack().Pop().Value()) + }) } - - t.Run("GetWitness", func(t *testing.T) { - w, err := c.GetWitness(ctr.ScriptHash()) - require.NoError(t, err) - v := newTestVM(w, tx) - require.NoError(t, v.Run()) - require.Equal(t, 1, v.Estack().Len()) - require.Equal(t, true, v.Estack().Pop().Value()) + t.Run("exact number of sigs", func(t *testing.T) { + testSigWit(t, 3) + }) + t.Run("larger number of sigs", func(t *testing.T) { + // Clean up. + var itm = c.Items[ctr.ScriptHash()] + for i := range itm.Parameters { + itm.Parameters[i].Value = nil + } + itm.Signatures = make(map[string][]byte) + testSigWit(t, 4) }) } From 773bcc3a5953bd4a50f577a5b447af8745da5a98 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 13:02:54 +0300 Subject: [PATCH 3/9] context: make error messages a bit less cryptic Refs. #2664. --- pkg/smartcontract/context/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/smartcontract/context/context.go b/pkg/smartcontract/context/context.go index cd4d09d0e..598149070 100644 --- a/pkg/smartcontract/context/context.go +++ b/pkg/smartcontract/context/context.go @@ -65,9 +65,9 @@ func (c *ParameterContext) GetWitness(h util.Uint160) (*transaction.Witness, err bw := io.NewBufBinWriter() for i := range item.Parameters { if item.Parameters[i].Type != smartcontract.SignatureType { - return nil, errors.New("only signature parameters are supported") + return nil, fmt.Errorf("unsupported %s parameter #%d", item.Parameters[i].Type.String(), i) } else if item.Parameters[i].Value == nil { - return nil, errors.New("nil parameter") + return nil, fmt.Errorf("no value for parameter #%d (not signed yet?)", i) } emit.Bytes(bw.BinWriter, item.Parameters[i].Value.([]byte)) } From c316107c9f7dae5d964399bf8df387f81fea8807 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 13:06:31 +0300 Subject: [PATCH 4/9] cli/wallet: avoid parsing address in signStoredTransaction It's not needed, we already have the hash and getDecryptedAccount can't return an account for a different one. --- cli/wallet/multisig.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index 2d9f34b2c..35f1a7db7 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -8,7 +8,6 @@ import ( "github.com/nspcc-dev/neo-go/cli/options" "github.com/nspcc-dev/neo-go/cli/paramcontext" "github.com/nspcc-dev/neo-go/pkg/core/transaction" - "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/urfave/cli" ) @@ -29,7 +28,9 @@ func signStoredTransaction(ctx *cli.Context) error { if !addrFlag.IsSet { return cli.NewExitError("address was not provided", 1) } - acc, err := getDecryptedAccount(wall, addrFlag.Uint160(), pass) + + var ch = addrFlag.Uint160() + acc, err := getDecryptedAccount(wall, ch, pass) if err != nil { return cli.NewExitError(err, 1) } @@ -39,10 +40,6 @@ func signStoredTransaction(ctx *cli.Context) error { return cli.NewExitError("verifiable item is not a transaction", 1) } - ch, err := address.StringToUint160(acc.Address) - if err != nil { - return cli.NewExitError(fmt.Errorf("wallet contains invalid account: %s", acc.Address), 1) - } signerFound := false for i := range tx.Signers { if tx.Signers[i].Account == ch { From c2c10c111c00f1639adf4c9eda8fc64c8dd278d9 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 15:43:30 +0300 Subject: [PATCH 5/9] cli/wallet: process non-out non-rpc calls to sign And document the behavior better. Fixes #2664. --- cli/multisig_test.go | 41 +++++++++++++++++++++++++++++++++++------ cli/wallet/multisig.go | 35 +++++++++++++++++++++++++---------- cli/wallet/wallet.go | 13 ++++++++++--- 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/cli/multisig_test.go b/cli/multisig_test.go index 7bf903c86..742822940 100644 --- a/cli/multisig_test.go +++ b/cli/multisig_test.go @@ -14,6 +14,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/neorpc/result" "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/context" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" @@ -162,13 +163,41 @@ func TestSignMultisigTx(t *testing.T) { require.Equal(t, vmstate.Halt.String(), res.State, res.FaultException) }) - e.In.WriteString("pass\r") - e.Run(t, "neo-go", "wallet", "sign", - "--rpc-endpoint", "http://"+e.RPC.Addr, - "--wallet", wallet2Path, "--address", multisigAddr, - "--in", txPath, "--out", txPath) - e.checkTxPersisted(t) + t.Run("console output", func(t *testing.T) { + oldIn, err := os.ReadFile(txPath) + require.NoError(t, err) + e.In.WriteString("pass\r") + e.Run(t, "neo-go", "wallet", "sign", + "--wallet", wallet2Path, "--address", multisigAddr, + "--in", txPath) + newIn, err := os.ReadFile(txPath) + require.NoError(t, err) + require.Equal(t, oldIn, newIn) + pcOld := new(context.ParameterContext) + require.NoError(t, json.Unmarshal(oldIn, pcOld)) + + jOut := e.Out.Bytes() + pcNew := new(context.ParameterContext) + require.NoError(t, json.Unmarshal(jOut, pcNew)) + + require.Equal(t, pcOld.Type, pcNew.Type) + require.Equal(t, pcOld.Network, pcNew.Network) + require.Equal(t, pcOld.Verifiable, pcNew.Verifiable) + require.Equal(t, pcOld.Items[multisigHash].Script, pcNew.Items[multisigHash].Script) + // It's completely signed after this, so parameters have signatures now as well. + require.NotEqual(t, pcOld.Items[multisigHash].Parameters, pcNew.Items[multisigHash].Parameters) + require.NotEqual(t, pcOld.Items[multisigHash].Signatures, pcNew.Items[multisigHash].Signatures) + }) + + t.Run("sign, save and send", func(t *testing.T) { + e.In.WriteString("pass\r") + e.Run(t, "neo-go", "wallet", "sign", + "--rpc-endpoint", "http://"+e.RPC.Addr, + "--wallet", wallet2Path, "--address", multisigAddr, + "--in", txPath, "--out", txPath) + e.checkTxPersisted(t) + }) t.Run("double-sign", func(t *testing.T) { e.In.WriteString("pass\r") e.RunWithError(t, "neo-go", "wallet", "sign", diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index 35f1a7db7..8807977d1 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -1,6 +1,7 @@ package wallet import ( + "encoding/json" "fmt" "github.com/nspcc-dev/neo-go/cli/cmdargs" @@ -12,6 +13,11 @@ import ( ) func signStoredTransaction(ctx *cli.Context) error { + var ( + out = ctx.String("out") + rpcNode = ctx.String(options.RPCEndpointFlag) + addrFlag = ctx.Generic("address").(*flags.Address) + ) if err := cmdargs.EnsureNone(ctx); err != nil { return err } @@ -20,11 +26,11 @@ func signStoredTransaction(ctx *cli.Context) error { return cli.NewExitError(err, 1) } - c, err := paramcontext.Read(ctx.String("in")) + pc, err := paramcontext.Read(ctx.String("in")) if err != nil { return cli.NewExitError(err, 1) } - addrFlag := ctx.Generic("address").(*flags.Address) + if !addrFlag.IsSet { return cli.NewExitError("address was not provided", 1) } @@ -35,7 +41,7 @@ func signStoredTransaction(ctx *cli.Context) error { return cli.NewExitError(err, 1) } - tx, ok := c.Verifiable.(*transaction.Transaction) + tx, ok := pc.Verifiable.(*transaction.Transaction) if !ok { return cli.NewExitError("verifiable item is not a transaction", 1) } @@ -52,18 +58,27 @@ func signStoredTransaction(ctx *cli.Context) error { } priv := acc.PrivateKey() - sign := priv.SignHashable(uint32(c.Network), tx) - if err := c.AddSignature(ch, acc.Contract, priv.PublicKey(), sign); err != nil { + sign := priv.SignHashable(uint32(pc.Network), tx) + if err := pc.AddSignature(ch, acc.Contract, priv.PublicKey(), sign); err != nil { return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) } - if out := ctx.String("out"); out != "" { - if err := paramcontext.Save(c, out); err != nil { - return cli.NewExitError(fmt.Errorf("failed to dump resulting transaction: %w", err), 1) + // Not saving and not sending, print. + if out == "" && rpcNode == "" { + txt, err := json.MarshalIndent(pc, " ", " ") + if err != nil { + return cli.NewExitError(fmt.Errorf("can't display resulting context: %w", err), 1) + } + fmt.Fprintln(ctx.App.Writer, string(txt)) + return nil + } + if out != "" { + if err := paramcontext.Save(pc, out); err != nil { + return cli.NewExitError(fmt.Errorf("can't save resulting context: %w", err), 1) } } - if len(ctx.String(options.RPCEndpointFlag)) != 0 { + if rpcNode != "" { for i := range tx.Signers { - w, err := c.GetWitness(tx.Signers[i].Account) + w, err := pc.GetWitness(tx.Signers[i].Account) if err != nil { return cli.NewExitError(fmt.Errorf("failed to construct witness for signer #%d: %w", i, err), 1) } diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index 42640245c..c74d358dd 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -281,9 +281,16 @@ func NewCommands() []cli.Command { { Name: "sign", Usage: "cosign transaction with multisig/contract/additional account", - UsageText: "sign -w wallet [--wallet-config path] --address
--in --out [-r ]", - Action: signStoredTransaction, - Flags: signFlags, + UsageText: "sign -w wallet [--wallet-config path] --address
--in [--out ] [-r ]", + Description: `Signs the given (in file.in) context (which must be a transaction + signing context) for the given address using the given wallet. This command can + output the resulting JSON (with additional signature added) right to the console + (if no file.out and no RPC endpoint specified) or into a file (which can be the + same as input one). If an RPC endpoint is given it'll also try to construct a + complete transaction and send it via RPC (printing its hash if everything is OK). +`, + Action: signStoredTransaction, + Flags: signFlags, }, { Name: "nep17", From a9237659ff08ce9c73857d155ab9c864aa36ff18 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 17:39:00 +0300 Subject: [PATCH 6/9] cli/wallet: correct error message It can be both NEP-11 and NEP-17. --- cli/wallet/nep17.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/wallet/nep17.go b/cli/wallet/nep17.go index 2185650fe..80623528b 100644 --- a/cli/wallet/nep17.go +++ b/cli/wallet/nep17.go @@ -271,7 +271,7 @@ func getNEPBalance(ctx *cli.Context, standard string, accHandler func(*cli.Conte // But if we have an exact hash, it must be correct. token, err = getTokenWithStandard(c, h, standard) if err != nil { - return cli.NewExitError(fmt.Errorf("%q is not a valid NEP-17 token: %w", name, err), 1) + return cli.NewExitError(fmt.Errorf("%q is not a valid %s token: %w", name, standard, err), 1) } } } From 2e6bd51727e284d5df9f2fcfedae4a7de1803ec0 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 21:12:08 +0300 Subject: [PATCH 7/9] cli/wallet: add strip-keys command To strip keys from wallets. --- cli/wallet/wallet.go | 40 ++++++++++++++++++++++++++++++++++++++++ cli/wallet_test.go | 30 ++++++++++++++++++++++++++++++ docs/cli.md | 5 +++++ 3 files changed, 75 insertions(+) diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index c74d358dd..b5f467683 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -292,6 +292,23 @@ func NewCommands() []cli.Command { Action: signStoredTransaction, Flags: signFlags, }, + { + Name: "strip-keys", + Usage: "remove private keys for all accounts", + UsageText: "neo-go wallet strip-keys -w wallet [--wallet-config path] [--force]", + Description: `Removes private keys for all accounts from the given wallet. Notice, + this is a very dangerous action (you can lose keys if you don't have a wallet + backup) that should not be performed unless you know what you're doing. It's + mostly useful for creation of special wallets that can be used to create + transactions, but can't be used to sign them (offline signing). +`, + Action: stripKeys, + Flags: []cli.Flag{ + walletPathFlag, + walletConfigFlag, + forceFlag, + }, + }, { Name: "nep17", Usage: "work with NEP-17 contracts", @@ -776,6 +793,29 @@ func dumpKeys(ctx *cli.Context) error { return nil } +func stripKeys(ctx *cli.Context) error { + if err := cmdargs.EnsureNone(ctx); err != nil { + return err + } + wall, _, err := readWallet(ctx) + if err != nil { + return cli.NewExitError(err, 1) + } + if !ctx.Bool("force") { + fmt.Fprintln(ctx.App.Writer, "All private keys for all accounts will be removed from the wallet. This action is irreversible.") + if ok := askForConsent(ctx.App.Writer); !ok { + return nil + } + } + for _, a := range wall.Accounts { + a.EncryptedWIF = "" + } + if err := wall.Save(); err != nil { + return cli.NewExitError(fmt.Errorf("error while saving wallet: %w", err), 1) + } + return nil +} + func createWallet(ctx *cli.Context) error { if err := cmdargs.EnsureNone(ctx); err != nil { return err diff --git a/cli/wallet_test.go b/cli/wallet_test.go index 09519bbb9..ea37fd5f6 100644 --- a/cli/wallet_test.go +++ b/cli/wallet_test.go @@ -630,6 +630,36 @@ func TestWalletImportDeployed(t *testing.T) { }) } +func TestStripKeys(t *testing.T) { + e := newExecutor(t, true) + tmpDir := t.TempDir() + walletPath := filepath.Join(tmpDir, "wallet.json") + e.In.WriteString("acc1\r") + e.In.WriteString("pass\r") + e.In.WriteString("pass\r") + e.Run(t, "neo-go", "wallet", "init", "--wallet", walletPath, "--account") + w1, err := wallet.NewWalletFromFile(walletPath) + require.NoError(t, err) + + e.RunWithError(t, "neo-go", "wallet", "strip-keys", "--wallet", walletPath, "something") + e.RunWithError(t, "neo-go", "wallet", "strip-keys", "--wallet", walletPath+".bad") + + e.In.WriteString("no") + e.Run(t, "neo-go", "wallet", "strip-keys", "--wallet", walletPath) + w2, err := wallet.NewWalletFromFile(walletPath) + require.NoError(t, err) + require.Equal(t, w1, w2) + + e.In.WriteString("y\r") + e.Run(t, "neo-go", "wallet", "strip-keys", "--wallet", walletPath) + e.Run(t, "neo-go", "wallet", "strip-keys", "--wallet", walletPath, "--force") // Does nothing effectively, but tests the force flag. + w3, err := wallet.NewWalletFromFile(walletPath) + require.NoError(t, err) + for _, a := range w3.Accounts { + require.Equal(t, "", a.EncryptedWIF) + } +} + func TestWalletDump(t *testing.T) { e := newExecutor(t, false) diff --git a/docs/cli.md b/docs/cli.md index 572b12c48..7825d87a2 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -272,6 +272,11 @@ transactions for this multisignature account with the imported key. contracts. They also can have WIF keys associated with them (in case your contract's `verify` method needs some signature). +#### Strip keys from accounts +`wallet strip-keys` allows you to remove private keys from the wallet, but let +it be used for other purposes (like creating transactions for subsequent +offline signing). Use with care, don't lose your keys with it. + ### Neo voting `wallet candidate` provides commands to register or unregister a committee (and therefore validator) candidate key: From f7cff022c0062e02fc8a49c2d2f8a444adff9b80 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 22:07:29 +0300 Subject: [PATCH 8/9] docs: mention txdump command which is very useful It wasn't documented. --- docs/cli.md | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/cli.md b/docs/cli.md index 7825d87a2..741769e96 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -524,7 +524,9 @@ If NEP-11 token supports optional `tokens` method, specify token hash via ./bin/neo-go wallet nep11 tokens -r http://localhost:20332 --token 67ecb7766dba4acf7c877392207984d1b4d15731 ``` -## Conversion utility +## Utility commands + +### Value conversion NeoGo provides conversion utility command to reverse data, convert script hashes to/from address, convert public keys to hashes/addresses, convert data to/from hexadecimal or base64 @@ -543,6 +545,66 @@ String to Hex 6465656537396331383966333030393862306261 String to Base64 ZGVlZTc5YzE4OWYzMDA5OGIwYmE2YTJlYjkwYjNhOTI1OGE2YzdmZg== ``` +### Transaction dumps/test invocations + +If you have a transaction signing context saved in a file (and many commands +like `wallet nep17 transfer` or `contract invokefunction` can give you one +with the `--out` parameter) you may want to check the contents before signing +it. This can be done with the `util txdump` command: +``` +$ ./bin/neo-go util txdump -r http://localhost:30333 some.part.json +Hash: f143059e0c03546db006608e0a0ad4b621b311a48d7fc62bb7062e405ab8e588 +OnChain: false +ValidUntil: 6004 +Signer: NVTiAjNgagDkTr5HTzDmQP9kPwPHN5BgVq (CalledByEntry) +SystemFee: 0.0208983 GAS +NetworkFee: 0.044159 GAS +Script: DCECs2Ir9AF73+MXxYrtX0x1PyBrfbiWBG+n13S7xL9/jcIRwBgSwB8MD2Rlc2lnbmF0ZUFzUm9sZQwU4pXjkVRMF4rZTwPsTc3/eFNOz0lBYn1bUg== +INDEX OPCODE PARAMETER +0 PUSHDATA1 02b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2 << +35 PUSH1 +36 PACK +37 PUSH8 +38 PUSH2 +39 PACK +40 PUSH15 +41 PUSHDATA1 64657369676e6174654173526f6c65 ("designateAsRole") +58 PUSHDATA1 e295e391544c178ad94f03ec4dcdff78534ecf49 +80 SYSCALL System.Contract.Call (627d5b52) +{ + "state": "HALT", + "gasconsumed": "2089830", + "script": "DCECs2Ir9AF73+MXxYrtX0x1PyBrfbiWBG+n13S7xL9/jcIRwBgSwB8MD2Rlc2lnbmF0ZUFzUm9sZQwU4pXjkVRMF4rZTwPsTc3/eFNOz0lBYn1bUg==", + "stack": [ + { + "type": "Any" + } + ], + "exception": null, + "notifications": [ + { + "contract": "0x49cf4e5378ffcd4dec034fd98a174c5491e395e2", + "eventname": "Designation", + "state": { + "type": "Array", + "value": [ + { + "type": "Integer", + "value": "8" + }, + { + "type": "Integer", + "value": "245" + } + ] + } + } + ] +} +``` +It always outputs the basic data and also can perform test-invocation if an +RPC endpoint is given to it. + ## VM CLI There is a VM CLI that you can use to load/analyze/run/step through some code: From 411ebdf51ed6e5583b33a5723987ed9ae303014b Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 31 Aug 2022 22:38:35 +0300 Subject: [PATCH 9/9] cli: add complete support for offline signing, fix #2662 See documentation update for an example. Some code is made generic as well, GetCompleteTransaction can now be used directly on ParameterContext. --- cli/paramcontext/context.go | 23 +++--- cli/util/convert.go | 12 +++ cli/util/send.go | 42 +++++++++++ cli/wallet/multisig.go | 21 +++--- cli/wallet/validator.go | 9 ++- cli/wallet_test.go | 92 +++++++++++++++++++++++ docs/cli.md | 74 ++++++++++++++++++ pkg/smartcontract/context/context.go | 18 +++++ pkg/smartcontract/context/context_test.go | 39 ++++++++-- 9 files changed, 303 insertions(+), 27 deletions(-) create mode 100644 cli/util/send.go diff --git a/cli/paramcontext/context.go b/cli/paramcontext/context.go index 5ebfe9efc..85175d4d2 100644 --- a/cli/paramcontext/context.go +++ b/cli/paramcontext/context.go @@ -13,18 +13,21 @@ import ( ) // InitAndSave creates an incompletely signed transaction which can be used -// as an input to `multisig sign`. +// as an input to `multisig sign`. If a wallet.Account is given and can sign, +// it's signed as well using it. func InitAndSave(net netmode.Magic, tx *transaction.Transaction, acc *wallet.Account, filename string) error { - priv := acc.PrivateKey() - pub := priv.PublicKey() - sign := priv.SignHashable(uint32(net), tx) scCtx := context.NewParameterContext("Neo.Network.P2P.Payloads.Transaction", net, tx) - h, err := address.StringToUint160(acc.Address) - if err != nil { - return fmt.Errorf("invalid address: %s", acc.Address) - } - if err := scCtx.AddSignature(h, acc.Contract, pub, sign); err != nil { - return fmt.Errorf("can't add signature: %w", err) + if acc != nil && acc.CanSign() { + priv := acc.PrivateKey() + pub := priv.PublicKey() + sign := priv.SignHashable(uint32(net), tx) + h, err := address.StringToUint160(acc.Address) + if err != nil { + return fmt.Errorf("invalid address: %s", acc.Address) + } + if err := scCtx.AddSignature(h, acc.Contract, pub, sign); err != nil { + return fmt.Errorf("can't add signature: %w", err) + } } return Save(scCtx, filename) } diff --git a/cli/util/convert.go b/cli/util/convert.go index 7fbd7a5bc..2f75be9bd 100644 --- a/cli/util/convert.go +++ b/cli/util/convert.go @@ -25,6 +25,18 @@ func NewCommands() []cli.Command { and converted to other formats. Strings are escaped and output in quotes.`, Action: handleParse, }, + { + Name: "sendtx", + Usage: "Send complete transaction stored in a context file", + UsageText: "sendtx [-r ] ", + Description: `Sends the transaction from the given context file to the given RPC node if it's + completely signed and ready. This command expects a ContractParametersContext + JSON file for input, it can't handle binary (or hex- or base64-encoded) + transactions. +`, + Action: sendTx, + Flags: txDumpFlags, + }, { Name: "txdump", Usage: "Dump transaction stored in file", diff --git a/cli/util/send.go b/cli/util/send.go new file mode 100644 index 000000000..59814c103 --- /dev/null +++ b/cli/util/send.go @@ -0,0 +1,42 @@ +package util + +import ( + "fmt" + + "github.com/nspcc-dev/neo-go/cli/options" + "github.com/nspcc-dev/neo-go/cli/paramcontext" + "github.com/urfave/cli" +) + +func sendTx(ctx *cli.Context) error { + args := ctx.Args() + if len(args) == 0 { + return cli.NewExitError("missing input file", 1) + } else if len(args) > 1 { + return cli.NewExitError("only one input file is accepted", 1) + } + + pc, err := paramcontext.Read(args[0]) + if err != nil { + return cli.NewExitError(err, 1) + } + + tx, err := pc.GetCompleteTransaction() + if err != nil { + return cli.NewExitError(fmt.Errorf("failed to complete transaction: %w", err), 1) + } + + gctx, cancel := options.GetTimeoutContext(ctx) + defer cancel() + + c, err := options.GetRPCClient(gctx, ctx) + if err != nil { + return cli.NewExitError(fmt.Errorf("failed to create RPC client: %w", err), 1) + } + res, err := c.SendRawTransaction(tx) + if err != nil { + return cli.NewExitError(fmt.Errorf("failed to submit transaction to RPC node: %w", err), 1) + } + fmt.Fprintln(ctx.App.Writer, res.StringLE()) + return nil +} diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index 8807977d1..114dcdf2f 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -57,10 +57,14 @@ func signStoredTransaction(ctx *cli.Context) error { return cli.NewExitError("tx signers don't contain provided account", 1) } - priv := acc.PrivateKey() - sign := priv.SignHashable(uint32(pc.Network), tx) - if err := pc.AddSignature(ch, acc.Contract, priv.PublicKey(), sign); err != nil { - return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) + if acc.CanSign() { + priv := acc.PrivateKey() + sign := priv.SignHashable(uint32(pc.Network), pc.Verifiable) + if err := pc.AddSignature(ch, acc.Contract, priv.PublicKey(), sign); err != nil { + return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) + } + } else if rpcNode == "" { + return cli.NewExitError(fmt.Errorf("can't sign transactions with the given account and no RPC endpoing given to send anything signed"), 1) } // Not saving and not sending, print. if out == "" && rpcNode == "" { @@ -77,12 +81,9 @@ func signStoredTransaction(ctx *cli.Context) error { } } if rpcNode != "" { - for i := range tx.Signers { - w, err := pc.GetWitness(tx.Signers[i].Account) - if err != nil { - return cli.NewExitError(fmt.Errorf("failed to construct witness for signer #%d: %w", i, err), 1) - } - tx.Scripts = append(tx.Scripts, *w) + tx, err = pc.GetCompleteTransaction() + if err != nil { + return cli.NewExitError(fmt.Errorf("failed to complete transaction: %w", err), 1) } gctx, cancel := options.GetTimeoutContext(ctx) diff --git a/cli/wallet/validator.go b/cli/wallet/validator.go index d4e832941..09717b46b 100644 --- a/cli/wallet/validator.go +++ b/cli/wallet/validator.go @@ -150,13 +150,20 @@ func handleVote(ctx *cli.Context) error { }) } -// getDecryptedAccount tries to unlock the specified account. If password is nil, it will be requested via terminal. +// getDecryptedAccount tries to get and unlock the specified account if it has a +// key inside (otherwise it's returned as is, without an ability to sign). If +// password is nil, it will be requested via terminal. func getDecryptedAccount(wall *wallet.Wallet, addr util.Uint160, password *string) (*wallet.Account, error) { acc := wall.GetAccount(addr) if acc == nil { return nil, fmt.Errorf("can't find account for the address: %s", address.Uint160ToString(addr)) } + // No private key available, nothing to decrypt, but it's still a useful account for many purposes. + if acc.EncryptedWIF == "" { + return acc, nil + } + if password == nil { pass, err := input.ReadPassword(EnterPasswordPrompt) if err != nil { diff --git a/cli/wallet_test.go b/cli/wallet_test.go index ea37fd5f6..d0393175c 100644 --- a/cli/wallet_test.go +++ b/cli/wallet_test.go @@ -660,6 +660,98 @@ func TestStripKeys(t *testing.T) { } } +func TestOfflineSigning(t *testing.T) { + e := newExecutor(t, true) + tmpDir := t.TempDir() + walletPath := filepath.Join(tmpDir, "wallet.json") + txPath := filepath.Join(tmpDir, "tx.json") + + // Copy wallet. + w, err := wallet.NewWalletFromFile(validatorWallet) + require.NoError(t, err) + jOut, err := w.JSON() + require.NoError(t, err) + require.NoError(t, os.WriteFile(walletPath, jOut, 0644)) + + // And remove keys from it. + e.Run(t, "neo-go", "wallet", "strip-keys", "--wallet", walletPath, "--force") + + t.Run("1/1 multisig", func(t *testing.T) { + args := []string{"neo-go", "wallet", "nep17", "transfer", + "--rpc-endpoint", "http://" + e.RPC.Addr, + "--wallet", walletPath, + "--from", validatorAddr, + "--to", w.Accounts[0].Address, + "--token", "NEO", + "--amount", "1", + "--force", + } + // walletPath has no keys, so this can't be sent. + e.RunWithError(t, args...) + // But can be saved. + e.Run(t, append(args, "--out", txPath)...) + // It can't be signed with the original wallet. + e.RunWithError(t, "neo-go", "wallet", "sign", + "--wallet", walletPath, "--address", validatorAddr, + "--in", txPath, "--out", txPath) + t.Run("sendtx", func(t *testing.T) { + // And it can't be sent. + e.RunWithError(t, "neo-go", "util", "sendtx", + "--rpc-endpoint", "http://"+e.RPC.Addr, + txPath) + // Even with too many arguments. + e.RunWithError(t, "neo-go", "util", "sendtx", + "--rpc-endpoint", "http://"+e.RPC.Addr, + txPath, txPath) + // Or no arguments at all. + e.RunWithError(t, "neo-go", "util", "sendtx", + "--rpc-endpoint", "http://"+e.RPC.Addr) + }) + // But it can be signed with a proper wallet. + e.In.WriteString("one\r") + e.Run(t, "neo-go", "wallet", "sign", + "--wallet", validatorWallet, "--address", validatorAddr, + "--in", txPath, "--out", txPath) + // And then anyone can send (even via wallet sign). + e.Run(t, "neo-go", "wallet", "sign", + "--rpc-endpoint", "http://"+e.RPC.Addr, + "--wallet", walletPath, "--address", validatorAddr, + "--in", txPath) + }) + e.checkTxPersisted(t) + t.Run("simple signature", func(t *testing.T) { + simpleAddr := w.Accounts[0].Address + args := []string{"neo-go", "wallet", "nep17", "transfer", + "--rpc-endpoint", "http://" + e.RPC.Addr, + "--wallet", walletPath, + "--from", simpleAddr, + "--to", validatorAddr, + "--token", "NEO", + "--amount", "1", + "--force", + } + // walletPath has no keys, so this can't be sent. + e.RunWithError(t, args...) + // But can be saved. + e.Run(t, append(args, "--out", txPath)...) + // It can't be signed with the original wallet. + e.RunWithError(t, "neo-go", "wallet", "sign", + "--wallet", walletPath, "--address", simpleAddr, + "--in", txPath, "--out", txPath) + // But can be with a proper one. + e.In.WriteString("one\r") + e.Run(t, "neo-go", "wallet", "sign", + "--wallet", validatorWallet, "--address", simpleAddr, + "--in", txPath, "--out", txPath) + // Sending without an RPC node is not likely to succeed. + e.RunWithError(t, "neo-go", "util", "sendtx", txPath) + // But it requires no wallet at all. + e.Run(t, "neo-go", "util", "sendtx", + "--rpc-endpoint", "http://"+e.RPC.Addr, + txPath) + }) +} + func TestWalletDump(t *testing.T) { e := newExecutor(t, false) diff --git a/docs/cli.md b/docs/cli.md index 741769e96..6d94abc73 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -382,6 +382,67 @@ $ ./bin/neo-go query voter -r http://localhost:20332 Nj91C8TxQSxW1jCE1ytFre6mg5q Block: 3970 ``` +### Transaction signing + +`wallet sign` command allows to sign arbitary transactions stored in JSON +format (also known as ContractParametersContext). Usually it's used in one of +the two cases: multisignature signing (when you don't have all keys for an +account and need to share the context with others until enough signatures +collected) or offline signing (when the node with a key is completely offline +and can't interact with the RPC node directly). + +#### Multisignature collection + +For example, you have a four-node default network setup and want to set some +key for the oracle role, you create transaction with: + +``` +$ neo-go contract invokefunction -w .docker/wallets/wallet1.json --out some.part.json -a NVTiAjNgagDkTr5HTzDmQP9kPwPHN5BgVq -r http://localhost:30333 0x49cf4e5378ffcd4dec034fd98a174c5491e395e2 designateAsRole 8 \[ 02b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2 \] -- NVTiAjNgagDkTr5HTzDmQP9kPwPHN5BgVq:CalledByEntry +``` + +And then sign it with two more keys: +``` +$ neo-go wallet sign -w .docker/wallets/wallet2.json --in some.part.json --out some.part.json -a NVTiAjNgagDkTr5HTzDmQP9kPwPHN5BgVq +$ neo-go wallet sign -w .docker/wallets/wallet3.json --in some.part.json -r http://localhost:30333 -a NVTiAjNgagDkTr5HTzDmQP9kPwPHN5BgVq +``` +Notice that the last command sends the transaction (which has a complete set +of singatures for 3/4 multisignature account by that time) to the network. + +#### Offline signing + +You want to do a transfer from a single-key account, but the key is on a +different (offline) machine. Create a stripped wallet first on the key-holding +machine: + +``` +$ cp wallet.json wallet.stripped.json # don't lose the original wallet +$ neo-go wallet strip-keys --wallet wallet.stripped.json +``` + +This wallet has no keys inside (but has appropriate scripts/addresses), so it +can be safely shared with anyone or transferred to network-enabled machine +where you then can create a transfer transaction: + +``` +$ neo-go wallet nep17 transfer --rpc-endpoint http://localhost:20332 \ + --wallet wallet.stripped.json --from NjEQfanGEXihz85eTnacQuhqhNnA6LxpLp \ + --to Nj91C8TxQSxW1jCE1ytFre6mg5qxTypg1Y --token NEO --amount 1 --out context.json + +``` +`context.json` can now be transferred to the machine with the `wallet.json` +containing proper keys and signed: +``` +$ neo-go wallet sign --wallet wallet.json \ + -address NjEQfanGEXihz85eTnacQuhqhNnA6LxpLp --in context.json --out context.json +``` +Now `context.json` contains a transaction with a complete set of signatures +(just one in this case, but of course you can do multisignature collection as +well). It can be transferred to network-enabled machine again and the +transaction can be sent to the network: +``` +$ neo-go util sendtx --rpc-endpoint http://localhost:20332 context.json +``` + ### NEP-17 token functions `wallet nep17` contains a set of commands to use for NEP-17 tokens. @@ -605,6 +666,19 @@ INDEX OPCODE PARAMETER It always outputs the basic data and also can perform test-invocation if an RPC endpoint is given to it. +### Sending signed transaction to the network + +If you have a completely finished (with all signatures collected) transaction +signing context saved in a file you can send it to the network (without any +wallet) using `util sendtx` command: +``` +$ ./bin/neo-go util sendtx -r http://localhost:30333 some.part.json +``` +This is useful in offline signing scenario, where the signing party doesn't +have any network access, so you can make a signature there, transfer the file +to another machine that has network access and then push the transaction out +to the network. + ## VM CLI There is a VM CLI that you can use to load/analyze/run/step through some code: diff --git a/pkg/smartcontract/context/context.go b/pkg/smartcontract/context/context.go index 598149070..eb307dbf8 100644 --- a/pkg/smartcontract/context/context.go +++ b/pkg/smartcontract/context/context.go @@ -56,6 +56,24 @@ func NewParameterContext(typ string, network netmode.Magic, verif crypto.Verifia } } +func (c *ParameterContext) GetCompleteTransaction() (*transaction.Transaction, error) { + tx, ok := c.Verifiable.(*transaction.Transaction) + if !ok { + return nil, errors.New("verifiable item is not a transaction") + } + if len(tx.Scripts) > 0 { + tx.Scripts = tx.Scripts[:0] + } + for i := range tx.Signers { + w, err := c.GetWitness(tx.Signers[i].Account) + if err != nil { + return nil, fmt.Errorf("can't create witness for signer #%d: %w", i, err) + } + tx.Scripts = append(tx.Scripts, *w) + } + return tx, nil +} + // GetWitness returns invocation and verification scripts for the specified contract. func (c *ParameterContext) GetWitness(h util.Uint160) (*transaction.Witness, error) { item, ok := c.Items[h] diff --git a/pkg/smartcontract/context/context_test.go b/pkg/smartcontract/context/context_test.go index c873b63e8..5675e0289 100644 --- a/pkg/smartcontract/context/context_test.go +++ b/pkg/smartcontract/context/context_test.go @@ -19,11 +19,17 @@ import ( "github.com/stretchr/testify/require" ) +type verifStub struct{} + +func (v verifStub) Hash() util.Uint256 { return util.Uint256{1, 2, 3} } +func (v verifStub) EncodeHashableFields() ([]byte, error) { return []byte{1}, nil } +func (v verifStub) DecodeHashableFields([]byte) error { return nil } + func TestParameterContext_AddSignatureSimpleContract(t *testing.T) { - tx := getContractTx() priv, err := keys.NewPrivateKey() require.NoError(t, err) pub := priv.PublicKey() + tx := getContractTx(pub.GetScriptHash()) sig := priv.SignHashable(uint32(netmode.UnitTestNet), tx) t.Run("invalid contract", func(t *testing.T) { @@ -75,9 +81,13 @@ func TestParameterContext_AddSignatureSimpleContract(t *testing.T) { }) } +func TestGetCompleteTransactionForNonTx(t *testing.T) { + c := NewParameterContext("Neo.Network.P2P.Payloads.Block", netmode.UnitTestNet, verifStub{}) + _, err := c.GetCompleteTransaction() + require.Error(t, err) +} + func TestParameterContext_AddSignatureMultisig(t *testing.T) { - tx := getContractTx() - c := NewParameterContext("Neo.Network.P2P.Payloads.Transaction", netmode.UnitTestNet, tx) privs, pubs := getPrivateKeys(t, 4) pubsCopy := keys.PublicKeys(pubs).Copy() script, err := smartcontract.CreateMultiSigRedeemScript(3, pubsCopy) @@ -91,6 +101,8 @@ func TestParameterContext_AddSignatureMultisig(t *testing.T) { newParam(smartcontract.SignatureType, "parameter2"), }, } + tx := getContractTx(ctr.ScriptHash()) + c := NewParameterContext("Neo.Network.P2P.Payloads.Transaction", netmode.UnitTestNet, tx) priv, err := keys.NewPrivateKey() require.NoError(t, err) sig := priv.SignHashable(uint32(c.Network), tx) @@ -98,6 +110,10 @@ func TestParameterContext_AddSignatureMultisig(t *testing.T) { indices := []int{2, 3, 0, 1} // random order testSigWit := func(t *testing.T, num int) { + t.Run("GetCompleteTransaction, bad", func(t *testing.T) { + _, err := c.GetCompleteTransaction() + require.Error(t, err) + }) for _, i := range indices[:num] { sig := privs[i].SignHashable(uint32(c.Network), tx) require.NoError(t, c.AddSignature(ctr.ScriptHash(), ctr, pubs[i], sig)) @@ -116,6 +132,17 @@ func TestParameterContext_AddSignatureMultisig(t *testing.T) { require.Equal(t, 1, v.Estack().Len()) require.Equal(t, true, v.Estack().Pop().Value()) }) + t.Run("GetCompleteTransaction, good", func(t *testing.T) { + tx, err := c.GetCompleteTransaction() + require.NoError(t, err) + require.Equal(t, 1, len(tx.Scripts)) + scripts1 := make([]transaction.Witness, len(tx.Scripts)) + copy(scripts1, tx.Scripts) + // Doing it twice shouldn't be a problem. + tx, err = c.GetCompleteTransaction() + require.NoError(t, err) + require.Equal(t, scripts1, tx.Scripts) + }) } t.Run("exact number of sigs", func(t *testing.T) { testSigWit(t, 3) @@ -143,7 +170,7 @@ func TestParameterContext_MarshalJSON(t *testing.T) { priv, err := keys.NewPrivateKey() require.NoError(t, err) - tx := getContractTx() + tx := getContractTx(priv.GetScriptHash()) sign := priv.SignHashable(uint32(netmode.UnitTestNet), tx) expected := &ParameterContext{ @@ -232,11 +259,11 @@ func newParam(typ smartcontract.ParamType, name string) wallet.ContractParam { } } -func getContractTx() *transaction.Transaction { +func getContractTx(signer util.Uint160) *transaction.Transaction { tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) tx.Attributes = make([]transaction.Attribute, 0) tx.Scripts = make([]transaction.Witness, 0) - tx.Signers = []transaction.Signer{{Account: util.Uint160{1, 2, 3}}} + tx.Signers = []transaction.Signer{{Account: signer}} tx.Hash() return tx }