accounts: add {Timed,}Unlock, remove SignLocked

accounts/account_manager.go

@@ -56,7 +56,6 @@ type Account struct {
 type Manager struct {
 	keyStore crypto.KeyStore2
 	unlocked map[string]*unlocked
-	unlockTime time.Duration
 	mutex    sync.RWMutex
 }
 
@@ -65,11 +64,10 @@ type unlocked struct {
 	abort chan struct{}
 }
 
-func NewManager(keyStore crypto.KeyStore2, unlockTime time.Duration) *Manager {
+func NewManager(keyStore crypto.KeyStore2) *Manager {
 	return &Manager{
 		keyStore: keyStore,
 		unlocked: make(map[string]*unlocked),
-		unlockTime: unlockTime,
 	}
 }
 
@@ -115,15 +113,28 @@ func (am *Manager) Sign(a Account, toSign []byte) (signature []byte, err error)
 	return signature, err
 }
 
-func (am *Manager) SignLocked(a Account, keyAuth string, toSign []byte) (signature []byte, err error) {
-	key, err := am.keyStore.GetKey(a.Address, keyAuth)
+// TimedUnlock unlocks the account with the given address.
+// When timeout has passed, the account will be locked again.
+func (am *Manager) TimedUnlock(addr []byte, keyAuth string, timeout time.Duration) error {
+	key, err := am.keyStore.GetKey(addr, keyAuth)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	u := am.addUnlocked(a.Address, key)
-	go am.dropLater(a.Address, u)
-	signature, err = crypto.Sign(toSign, key.PrivateKey)
-	return signature, err
+	u := am.addUnlocked(addr, key)
+	go am.dropLater(addr, u, timeout)
+	return nil
+}
+
+// Unlock unlocks the account with the given address. The account
+// stays unlocked until the program exits or until a TimedUnlock
+// timeout (started after the call to Unlock) expires.
+func (am *Manager) Unlock(addr []byte, keyAuth string) error {
+	key, err := am.keyStore.GetKey(addr, keyAuth)
+	if err != nil {
+		return err
+	}
+	am.addUnlocked(addr, key)
+	return nil
 }
 
 func (am *Manager) NewAccount(auth string) (Account, error) {
@@ -155,6 +166,9 @@ func (am *Manager) addUnlocked(addr []byte, key *crypto.Key) *unlocked {
 	if found {
 		// terminate dropLater for this key to avoid unexpected drops.
 		close(prev.abort)
+		// the key is zeroed here instead of in dropLater because
+		// there might not actually be a dropLater running for this
+		// key, i.e. when Unlock was used.
 		zeroKey(prev.PrivateKey)
 	}
 	am.unlocked[string(addr)] = u
@@ -162,8 +176,8 @@ func (am *Manager) addUnlocked(addr []byte, key *crypto.Key) *unlocked {
 	return u
 }
 
-func (am *Manager) dropLater(addr []byte, u *unlocked) {
-	t := time.NewTimer(am.unlockTime)
+func (am *Manager) dropLater(addr []byte, u *unlocked, timeout time.Duration) {
+	t := time.NewTimer(timeout)
 	defer t.Stop()
 	select {
 	case <-u.abort:

accounts/accounts_test.go

@@ -1,44 +1,36 @@
 package accounts
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
-
 	"time"
 
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/crypto/randentropy"
-	"github.com/ethereum/go-ethereum/ethutil"
 )
 
-func TestAccountManager(t *testing.T) {
-	ks := crypto.NewKeyStorePlain(ethutil.DefaultDataDir() + "/testaccounts")
-	am := NewManager(ks, 100*time.Millisecond)
+func TestSign(t *testing.T) {
+	dir, ks := tmpKeyStore(t, crypto.NewKeyStorePlain)
+	defer os.RemoveAll(dir)
+
+	am := NewManager(ks)
 	pass := "" // not used but required by API
 	a1, err := am.NewAccount(pass)
 	toSign := randentropy.GetEntropyCSPRNG(32)
-	_, err = am.SignLocked(a1, pass, toSign)
-	if err != nil {
-		t.Fatal(err)
-	}
+	am.Unlock(a1.Address, "")
 
-	// Cleanup
-	time.Sleep(150 * time.Millisecond) // wait for locking
-
-	accounts, err := am.Accounts()
-	if err != nil {
-		t.Fatal(err)
-	}
-	for _, account := range accounts {
-		err := am.DeleteAccount(account.Address, pass)
+	_, err = am.Sign(a1, toSign)
 	if err != nil {
 		t.Fatal(err)
 	}
 }
-}
 
-func TestAccountManagerLocking(t *testing.T) {
-	ks := crypto.NewKeyStorePassphrase(ethutil.DefaultDataDir() + "/testaccounts")
-	am := NewManager(ks, 200*time.Millisecond)
+func TestTimedUnlock(t *testing.T) {
+	dir, ks := tmpKeyStore(t, crypto.NewKeyStorePassphrase)
+	defer os.RemoveAll(dir)
+
+	am := NewManager(ks)
 	pass := "foo"
 	a1, err := am.NewAccount(pass)
 	toSign := randentropy.GetEntropyCSPRNG(32)
@@ -46,38 +38,32 @@ func TestAccountManagerLocking(t *testing.T) {
 	// Signing without passphrase fails because account is locked
 	_, err = am.Sign(a1, toSign)
 	if err != ErrLocked {
-		t.Fatal(err)
+		t.Fatal("Signing should've failed with ErrLocked before unlocking, got ", err)
 	}
 
 	// Signing with passphrase works
-	_, err = am.SignLocked(a1, pass, toSign)
-	if err != nil {
+	if err = am.TimedUnlock(a1.Address, pass, 100*time.Millisecond); err != nil {
 		t.Fatal(err)
 	}
 
 	// Signing without passphrase works because account is temp unlocked
 	_, err = am.Sign(a1, toSign)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatal("Signing shouldn't return an error after unlocking, got ", err)
 	}
 
-	// Signing without passphrase fails after automatic locking
-	time.Sleep(250 * time.Millisecond)
-
+	// Signing fails again after automatic locking
+	time.Sleep(150 * time.Millisecond)
 	_, err = am.Sign(a1, toSign)
 	if err != ErrLocked {
-		t.Fatal(err)
+		t.Fatal("Signing should've failed with ErrLocked timeout expired, got ", err)
+	}
 }
 
-	// Cleanup
-	accounts, err := am.Accounts()
+func tmpKeyStore(t *testing.T, new func(string) crypto.KeyStore2) (string, crypto.KeyStore2) {
+	d, err := ioutil.TempDir("", "eth-keystore-test")
 	if err != nil {
 		t.Fatal(err)
 	}
-	for _, account := range accounts {
-		err := am.DeleteAccount(account.Address, pass)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
+	return d, new(d)
 }

cmd/utils/flags.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"path"
 	"runtime"
-	"time"
 
 	"github.com/codegangsta/cli"
 	"github.com/ethereum/go-ethereum/accounts"
@@ -199,7 +198,7 @@ func GetChain(ctx *cli.Context) (*core.ChainManager, ethutil.Database, ethutil.D
 func GetAccountManager(ctx *cli.Context) *accounts.Manager {
 	dataDir := ctx.GlobalString(DataDirFlag.Name)
 	ks := crypto.NewKeyStorePassphrase(path.Join(dataDir, "keys"))
-	return accounts.NewManager(ks, 300*time.Second)
+	return accounts.NewManager(ks)
 }
 
 func StartRPC(eth *eth.Ethereum, ctx *cli.Context) {