overhaul SignTransaction

In a nutshell, SignTransaction now does less work: it requires the
user to fill out the UnlockConditions and TransactionSignatures of
the transaction, whereas before it would fill them in itself. This
is a better approach because it makes the most common operation --
signing all the inputs that the wallet controls -- dead simple. And
requiring the user to fill out the unlock conditions isn't a big
deal, because they can get those from /wallet/unspent. Lastly, if
the user is responsible for filling out the TransactionSignature
fields, they can control precisely what gets signed.

cmd/siac/walletcmd.go

@@ -14,6 +14,7 @@ import (
 	"github.com/spf13/cobra"
 	"golang.org/x/crypto/ssh/terminal"
 
+	"github.com/NebulousLabs/Sia/crypto"
 	"github.com/NebulousLabs/Sia/encoding"
 	"github.com/NebulousLabs/Sia/modules"
 	"github.com/NebulousLabs/Sia/modules/wallet"
@@ -167,7 +168,7 @@ Run 'wallet send --help' to see a list of available units.`,
 		Long: `Sign the specified inputs of a transaction. If siad is running with an
 unlocked wallet, the /wallet/sign API call will be used. Otherwise, sign will
 prompt for the wallet seed, and the signing key(s) will be regenerated.`,
-		Run: wrap(walletsigncmd),
+		Run: walletsigncmd,
 	}
 
 	walletSweepCmd = &cobra.Command{
@@ -510,17 +511,24 @@ func walletsweepcmd() {
 }
 
 // walletsigncmd signs a transaction.
-func walletsigncmd(txnJSON, toSignJSON string) {
+func walletsigncmd(cmd *cobra.Command, args []string) {
+	if len(args) < 1 || len(args) > 2 {
+		cmd.UsageFunc()(cmd)
+		os.Exit(exitCodeUsage)
+	}
+
 	var txn types.Transaction
-	err := json.Unmarshal([]byte(txnJSON), &txn)
+	err := json.Unmarshal([]byte(args[0]), &txn)
 	if err != nil {
 		die("Invalid transaction:", err)
 	}
 
-	var toSign map[types.OutputID]types.UnlockHash
-	err = json.Unmarshal([]byte(toSignJSON), &toSign)
-	if err != nil {
-		die("Invalid transaction:", err)
+	var toSign []crypto.Hash
+	if len(args) == 2 {
+		err = json.Unmarshal([]byte(args[1]), &toSign)
+		if err != nil {
+			die("Invalid transaction:", err)
+		}
 	}
 
 	// try API first

doc/API.md

@@ -1543,11 +1543,10 @@ specified.
 ```
 {
   "transaction": { }, // types.Transaction
-  "tosign": {
-    // types.OutputID -> types.UnlockHash
-    "3689bd3489679aabcde02e01345abcde": "138950f0129d74acd4eade3453b45678",
-    "132cee478a9bb98bdd23cf05376cdf2a": "7cbcd123578234ce0f12fe01a68ba9bf"
-  }
+  "tosign": [
+    "1234567890abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+    "abcdef0123456789abcdef0123456789abcd1234567890ef0123456789abcdef"
+  ]
 }
 ```
 

doc/api/Wallet.md

@@ -514,8 +514,8 @@ specified.
 
   // inputs to sign; a mapping from OutputID to UnlockHash
   "tosign": {
-    "3689bd3489679aabcde02e01345abcde": "138950f0129d74acd4eade3453b45678",
-    "132cee478a9bb98bdd23cf05376cdf2a": "7cbcd123578234ce0f12fe01a68ba9bf"
+    "1234567890abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+    "abcdef0123456789abcdef0123456789abcd1234567890ef0123456789abcdef"
   }
 }
 ```

modules/wallet.go

@@ -431,11 +431,10 @@ type (
 		// SpendableOutputs returns the outputs spendable by the wallet.
 		SpendableOutputs() []SpendableOutput
 
-		// SignTransaction signs txn using secret keys known to the wallet. toSign
-		// maps the ParentID of each unsigned input to the UnlockHash of that input's
-		// desired UnlockConditions. SignTransaction fills in the UnlockConditions for
-		// each such input and adds a corresponding signature.
-		SignTransaction(txn *types.Transaction, toSign map[types.OutputID]types.UnlockHash) error
+		// SignTransaction signs txn using secret keys known to the wallet.
+		// The transaction should be complete with the exception of the
+		// Signature fields of each TransactionSignature referenced by toSign.
+		SignTransaction(txn *types.Transaction, toSign []crypto.Hash) error
 	}
 
 	// WalletSettings control the behavior of the Wallet.

modules/wallet/transactionbuilder.go

@@ -775,28 +775,46 @@ outer:
 	return outputs
 }
 
-// SignTransaction signs txn using secret keys known to the wallet. toSign
-// maps the ParentID of each unsigned input to the UnlockHash of that input's
-// desired UnlockConditions. SignTransaction fills in the UnlockConditions for
-// each such input and adds a corresponding signature.
-func (w *Wallet) SignTransaction(txn *types.Transaction, toSign map[types.OutputID]types.UnlockHash) error {
+// SignTransaction signs txn using secret keys known to the wallet. The
+// transaction should be complete with the exception of the Signature fields
+// of each TransactionSignature referenced by toSign. For convenience, if
+// toSign is empty, SignTransaction signs everything that it can.
+func (w *Wallet) SignTransaction(txn *types.Transaction, toSign []crypto.Hash) error {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 	if !w.unlocked {
 		return modules.ErrLockedWallet
 	}
+	// if toSign is empty, sign all inputs that we have keys for
+	if len(toSign) == 0 {
+		for _, sci := range txn.SiacoinInputs {
+			if _, ok := w.keys[sci.UnlockConditions.UnlockHash()]; ok {
+				toSign = append(toSign, crypto.Hash(sci.ParentID))
+			}
+		}
+		for _, sfi := range txn.SiafundInputs {
+			if _, ok := w.keys[sfi.UnlockConditions.UnlockHash()]; ok {
+				toSign = append(toSign, crypto.Hash(sfi.ParentID))
+			}
+		}
+	}
 	return signTransaction(txn, w.keys, toSign)
 }
 
-// SignTransaction signs txn using secret keys derived from seed. toSign maps
-// the ParentID of each unsigned input to the UnlockHash of that input's
-// desired UnlockConditions. SignTransaction fills in the UnlockConditions for
-// each such input and adds a corresponding signature.
+// SignTransaction signs txn using secret keys derived from seed. The
+// transaction should be complete with the exception of the Signature fields
+// of each TransactionSignature referenced by toSign, which must not be empty.
 //
 // SignTransaction must derive all of the keys from scratch, so it is
 // appreciably slower than calling the Wallet.SignTransaction method. Only the
 // first 1 million keys are derived.
-func SignTransaction(txn *types.Transaction, seed modules.Seed, toSign map[types.OutputID]types.UnlockHash) error {
+func SignTransaction(txn *types.Transaction, seed modules.Seed, toSign []crypto.Hash) error {
+	if len(toSign) == 0 {
+		// unlike the wallet method, we can't simply "sign all inputs we have
+		// keys for," because without generating all of the keys up front, we
+		// don't know how many inputs we actually have keys for.
+		return errors.New("toSign cannot be empty")
+	}
 	// generate keys in batches up to 1e6 before giving up
 	keys := make(map[types.UnlockHash]spendableKey, 1e6)
 	var keyIndex uint64
@@ -815,42 +833,73 @@ func SignTransaction(txn *types.Transaction, seed modules.Seed, toSign map[types
 
 // signTransaction signs the specified inputs of txn using the specified keys.
 // It returns an error if any of the specified inputs cannot be signed.
-func signTransaction(txn *types.Transaction, keys map[types.UnlockHash]spendableKey, toSign map[types.OutputID]types.UnlockHash) error {
-	signed := 0
-	for i, sci := range txn.SiacoinInputs {
-		uh, ok := toSign[types.OutputID(sci.ParentID)]
-		if !ok {
-			// not signing this input
-			continue
+func signTransaction(txn *types.Transaction, keys map[types.UnlockHash]spendableKey, toSign []crypto.Hash) error {
+	// helper function to lookup unlock conditions in the txn associated with
+	// a transaction signature's ParentID
+	findUnlockConditions := func(id crypto.Hash) (types.UnlockConditions, bool) {
+		for _, sci := range txn.SiacoinInputs {
+			if crypto.Hash(sci.ParentID) == id {
+				return sci.UnlockConditions, true
+			}
 		}
-		// lookup the signing key(s)
-		sk, ok := keys[uh]
-		if !ok {
-			return errors.New("could not locate signing key for " + uh.String())
+		for _, sfi := range txn.SiafundInputs {
+			if crypto.Hash(sfi.ParentID) == id {
+				return sfi.UnlockConditions, true
+			}
 		}
-		txn.SiacoinInputs[i].UnlockConditions = sk.UnlockConditions
-		cf := types.CoveredFields{WholeTransaction: true}
-		addSignatures(txn, cf, sk.UnlockConditions, crypto.Hash(sci.ParentID), sk)
-		signed++
+		return types.UnlockConditions{}, false
 	}
-	for i, sfi := range txn.SiafundInputs {
-		uh, ok := toSign[types.OutputID(sfi.ParentID)]
+	// helper function to lookup the secret key that can sign
+	findSigningKey := func(uc types.UnlockConditions, pubkeyIndex uint64) (crypto.SecretKey, bool) {
+		if pubkeyIndex >= uint64(len(uc.PublicKeys)) {
+			return crypto.SecretKey{}, false
+		}
+		pk := uc.PublicKeys[pubkeyIndex]
+		sk, ok := keys[uc.UnlockHash()]
 		if !ok {
-			// not signing this input
-			continue
+			return crypto.SecretKey{}, false
+		}
+		for _, key := range sk.SecretKeys {
+			pubKey := key.PublicKey()
+			if bytes.Equal(pk.Key, pubKey[:]) {
+				return key, true
+			}
+		}
+		return crypto.SecretKey{}, false
+	}
+
+	for _, id := range toSign {
+		// find associated txn signature
+		//
+		// NOTE: it's possible that the Signature field will already be filled
+		// out. Although we could save a bit of work by not signing it, in
+		// practice it's probably best to overwrite any existing signatures,
+		// since we know that ours will be valid.
+		sigIndex := -1
+		for i, sig := range txn.TransactionSignatures {
+			if sig.ParentID == id {
+				sigIndex = i
+				break
+			}
+		}
+		if sigIndex == -1 {
+			return errors.New("toSign references signatures not present in transaction")
 		}
-		// lookup the signing key(s)
-		sk, ok := keys[uh]
+		// find associated input
+		uc, ok := findUnlockConditions(id)
 		if !ok {
-			return errors.New("could not locate signing key for " + uh.String())
+			return errors.New("toSign references IDs not present in transaction")
 		}
-		txn.SiafundInputs[i].UnlockConditions = sk.UnlockConditions
-		cf := types.CoveredFields{WholeTransaction: true}
-		addSignatures(txn, cf, sk.UnlockConditions, crypto.Hash(sfi.ParentID), sk)
-		signed++
-	}
-	if signed != len(toSign) {
-		return errors.New("toSign references OutputIDs not present in transaction")
+		// lookup the signing key
+		sk, ok := findSigningKey(uc, txn.TransactionSignatures[sigIndex].PublicKeyIndex)
+		if !ok {
+			return errors.New("could not locate signing key for " + id.String())
+		}
+		// add signature
+		sigHash := txn.SigHash(sigIndex)
+		encodedSig := crypto.SignHash(sigHash, sk)
+		txn.TransactionSignatures[sigIndex].Signature = encodedSig[:]
 	}
+
 	return nil
 }

modules/wallet/transactionbuilder_test.go

@@ -4,6 +4,7 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/NebulousLabs/Sia/crypto"
 	"github.com/NebulousLabs/Sia/modules"
 	"github.com/NebulousLabs/Sia/types"
 )
@@ -526,18 +527,20 @@ func TestSignTransaction(t *testing.T) {
 	// create a transaction that sends an output to the void
 	txn := types.Transaction{
 		SiacoinInputs: []types.SiacoinInput{{
-			ParentID: types.SiacoinOutputID(outputs[0].ID),
+			ParentID:         types.SiacoinOutputID(outputs[0].ID),
+			UnlockConditions: outputs[0].UnlockConditions,
 		}},
 		SiacoinOutputs: []types.SiacoinOutput{{
 			Value:      outputs[0].Value,
 			UnlockHash: types.UnlockHash{},
 		}},
+		TransactionSignatures: []types.TransactionSignature{{
+			ParentID: crypto.Hash(outputs[0].ID),
+		}},
 	}
 
 	// sign the transaction
-	err = wt.wallet.SignTransaction(&txn, map[types.OutputID]types.UnlockHash{
-		outputs[0].ID: outputs[0].UnlockHash,
-	})
+	err = wt.wallet.SignTransaction(&txn, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

node/api/client/wallet.go

@@ -7,6 +7,7 @@ import (
 	"net/url"
 	"strconv"
 
+	"github.com/NebulousLabs/Sia/crypto"
 	"github.com/NebulousLabs/Sia/node/api"
 	"github.com/NebulousLabs/Sia/types"
 )
@@ -108,7 +109,7 @@ func (c *Client) WalletSiacoinsPost(amount types.Currency, destination types.Unl
 }
 
 // WalletSignPost uses the /wallet/sign api endpoint to sign a transaction.
-func (c *Client) WalletSignPost(txn types.Transaction, toSign map[types.OutputID]types.UnlockHash) (wspr api.WalletSignPOSTResp, err error) {
+func (c *Client) WalletSignPost(txn types.Transaction, toSign []crypto.Hash) (wspr api.WalletSignPOSTResp, err error) {
 	buf := new(bytes.Buffer)
 	err = json.NewEncoder(buf).Encode(api.WalletSignPOSTParams{
 		Transaction: txn,

node/api/wallet.go

@@ -68,8 +68,8 @@ type (
 	// WalletSignPOSTParams contains the unsigned transaction and a set of
 	// inputs to sign.
 	WalletSignPOSTParams struct {
-		Transaction types.Transaction                   `json:"transaction"`
-		ToSign      map[types.OutputID]types.UnlockHash `json:"tosign"`
+		Transaction types.Transaction `json:"transaction"`
+		ToSign      []crypto.Hash     `json:"tosign"`
 	}
 
 	// WalletSignPOSTResp contains the signed transaction.

siatest/wallet/wallet_test.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/NebulousLabs/Sia/build"
+	"github.com/NebulousLabs/Sia/crypto"
 	"github.com/NebulousLabs/Sia/node"
 	"github.com/NebulousLabs/Sia/siatest"
 	"github.com/NebulousLabs/Sia/types"
@@ -145,18 +146,20 @@ func TestSignTransaction(t *testing.T) {
 	// create a transaction that sends an output to the void
 	txn := types.Transaction{
 		SiacoinInputs: []types.SiacoinInput{{
-			ParentID: types.SiacoinOutputID(outputs[0].ID),
+			ParentID:         types.SiacoinOutputID(outputs[0].ID),
+			UnlockConditions: outputs[0].UnlockConditions,
 		}},
 		SiacoinOutputs: []types.SiacoinOutput{{
 			Value:      outputs[0].Value,
 			UnlockHash: types.UnlockHash{},
 		}},
+		TransactionSignatures: []types.TransactionSignature{{
+			ParentID: crypto.Hash(outputs[0].ID),
+		}},
 	}
 
 	// sign the transaction
-	signResp, err := testNode.WalletSignPost(txn, map[types.OutputID]types.UnlockHash{
-		outputs[0].ID: outputs[0].UnlockHash,
-	})
+	signResp, err := testNode.WalletSignPost(txn, nil)
 	if err != nil {
 		t.Fatal("failed to sign the transaction", err)
 	}