Skip to content

Commit

Permalink
Merge pull request #2668 from nspcc-dev/cli-signing-improvements
Browse files Browse the repository at this point in the history
CLI signing improvements
  • Loading branch information
roman-khimov authored Sep 1, 2022
2 parents 314cd33 + 411ebdf commit 20224cb
Show file tree
Hide file tree
Showing 13 changed files with 556 additions and 79 deletions.
41 changes: 35 additions & 6 deletions cli/multisig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down
23 changes: 13 additions & 10 deletions cli/paramcontext/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 4 additions & 7 deletions cli/smartcontract/smart_contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions cli/util/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <endpoint>] <file.in>",
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",
Expand Down
42 changes: 42 additions & 0 deletions cli/util/send.go
Original file line number Diff line number Diff line change
@@ -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
}
59 changes: 36 additions & 23 deletions cli/wallet/multisig.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
package wallet

import (
"encoding/json"
"fmt"

"github.com/nspcc-dev/neo-go/cli/cmdargs"
"github.com/nspcc-dev/neo-go/cli/flags"
"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"
)

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
}
Expand All @@ -21,28 +26,26 @@ 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)
}
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)
}

tx, ok := c.Verifiable.(*transaction.Transaction)
tx, ok := pc.Verifiable.(*transaction.Transaction)
if !ok {
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 {
Expand All @@ -54,23 +57,33 @@ func signStoredTransaction(ctx *cli.Context) error {
return cli.NewExitError("tx signers don't contain provided account", 1)
}

priv := acc.PrivateKey()
sign := priv.SignHashable(uint32(c.Network), tx)
if err := c.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)
}
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 len(ctx.String(options.RPCEndpointFlag)) != 0 {
for i := range tx.Signers {
w, err := c.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)
if out != "" {
if err := paramcontext.Save(pc, out); err != nil {
return cli.NewExitError(fmt.Errorf("can't save resulting context: %w", err), 1)
}
}
if rpcNode != "" {
tx, err = pc.GetCompleteTransaction()
if err != nil {
return cli.NewExitError(fmt.Errorf("failed to complete transaction: %w", err), 1)
}

gctx, cancel := options.GetTimeoutContext(ctx)
Expand Down
2 changes: 1 addition & 1 deletion cli/wallet/nep17.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion cli/wallet/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
53 changes: 50 additions & 3 deletions cli/wallet/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,33 @@ func NewCommands() []cli.Command {
{
Name: "sign",
Usage: "cosign transaction with multisig/contract/additional account",
UsageText: "sign -w wallet [--wallet-config path] --address <address> --in <file.in> --out <file.out> [-r <endpoint>]",
Action: signStoredTransaction,
Flags: signFlags,
UsageText: "sign -w wallet [--wallet-config path] --address <address> --in <file.in> [--out <file.out>] [-r <endpoint>]",
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: "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",
Expand Down Expand Up @@ -769,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
Expand Down
Loading

0 comments on commit 20224cb

Please sign in to comment.