accounts: streamline API

- Manager.Accounts no longer returns an error.
- Manager methods take Account instead of common.Address.
- All uses of Account with unkeyed fields are converted.

accounts/account_manager.go

@@ -24,7 +24,6 @@ import (
 	crand "crypto/rand"
 	"errors"
 	"fmt"
-	"os"
 	"sync"
 	"time"
 
@@ -70,8 +69,8 @@ func NewPlaintextManager(keydir string) *Manager {
 	}
 }
 
-func (am *Manager) HasAccount(addr common.Address) bool {
+func (am *Manager) HasAddress(addr common.Address) bool {
-	accounts, _ := am.Accounts()
+	accounts := am.Accounts()
 	for _, acct := range accounts {
 		if acct.Address == addr {
 			return true
@@ -80,8 +79,8 @@ func (am *Manager) HasAccount(addr common.Address) bool {
 	return false
 }
 
-func (am *Manager) DeleteAccount(address common.Address, auth string) error {
+func (am *Manager) DeleteAccount(a Account, auth string) error {
-	return am.keyStore.DeleteKey(address, auth)
+	return am.keyStore.DeleteKey(a.Address, auth)
 }
 
 func (am *Manager) Sign(a Account, toSign []byte) (signature []byte, err error) {
@@ -96,8 +95,8 @@ func (am *Manager) Sign(a Account, toSign []byte) (signature []byte, err error)
 }
 
 // Unlock unlocks the given account indefinitely.
-func (am *Manager) Unlock(addr common.Address, keyAuth string) error {
+func (am *Manager) Unlock(a Account, keyAuth string) error {
-	return am.TimedUnlock(addr, keyAuth, 0)
+	return am.TimedUnlock(a, keyAuth, 0)
 }
 
 func (am *Manager) Lock(addr common.Address) error {
@@ -117,8 +116,8 @@ func (am *Manager) Lock(addr common.Address) error {
 //
 // If the accout is already unlocked, TimedUnlock extends or shortens
 // the active unlock timeout.
-func (am *Manager) TimedUnlock(addr common.Address, keyAuth string, timeout time.Duration) error {
+func (am *Manager) TimedUnlock(a Account, keyAuth string, timeout time.Duration) error {
-	key, err := am.keyStore.GetKey(addr, keyAuth)
+	key, err := am.keyStore.GetKey(a.Address, keyAuth)
 	if err != nil {
 		return err
 	}
@@ -126,7 +125,7 @@ func (am *Manager) TimedUnlock(addr common.Address, keyAuth string, timeout time
 	am.mutex.Lock()
 	defer am.mutex.Unlock()
 	var found bool
-	u, found = am.unlocked[addr]
+	u, found = am.unlocked[a.Address]
 	if found {
 		// terminate dropLater for this key to avoid unexpected drops.
 		if u.abort != nil {
@@ -135,11 +134,11 @@ func (am *Manager) TimedUnlock(addr common.Address, keyAuth string, timeout time
 	}
 	if timeout > 0 {
 		u = &unlocked{Key: key, abort: make(chan struct{})}
-		go am.expire(addr, u, timeout)
+		go am.expire(a.Address, u, timeout)
 	} else {
 		u = &unlocked{Key: key}
 	}
-	am.unlocked[addr] = u
+	am.unlocked[a.Address] = u
 	return nil
 }
 
@@ -171,34 +170,26 @@ func (am *Manager) NewAccount(auth string) (Account, error) {
 	return Account{Address: key.Address}, nil
 }
 
-func (am *Manager) AddressByIndex(index int) (addr string, err error) {
+func (am *Manager) AccountByIndex(index int) (Account, error) {
-	var addrs []common.Address
+	addrs, err := am.keyStore.GetKeyAddresses()
-	addrs, err = am.keyStore.GetKeyAddresses()
 	if err != nil {
-		return
+		return Account{}, err
 	}
 	if index < 0 || index >= len(addrs) {
-		err = fmt.Errorf("index out of range: %d (should be 0-%d)", index, len(addrs)-1)
+		return Account{}, fmt.Errorf("account index %d not in range [0, %d]", index, len(addrs)-1)
-	} else {
-		addr = addrs[index].Hex()
 	}
-	return
+	return Account{Address: addrs[index]}, nil
 }
 
-func (am *Manager) Accounts() ([]Account, error) {
+func (am *Manager) Accounts() []Account {
-	addresses, err := am.keyStore.GetKeyAddresses()
+	addresses, _ := am.keyStore.GetKeyAddresses()
-	if os.IsNotExist(err) {
-		return nil, ErrNoKeys
-	} else if err != nil {
-		return nil, err
-	}
 	accounts := make([]Account, len(addresses))
 	for i, addr := range addresses {
 		accounts[i] = Account{
 			Address: addr,
 		}
 	}
-	return accounts, err
+	return accounts
 }
 
 // zeroKey zeroes a private key in memory.
@@ -211,8 +202,8 @@ func zeroKey(k *ecdsa.PrivateKey) {
 
 // USE WITH CAUTION = this will save an unencrypted private key on disk
 // no cli or js interface
-func (am *Manager) Export(path string, addr common.Address, keyAuth string) error {
+func (am *Manager) Export(path string, a Account, keyAuth string) error {
-	key, err := am.keyStore.GetKey(addr, keyAuth)
+	key, err := am.keyStore.GetKey(a.Address, keyAuth)
 	if err != nil {
 		return err
 	}
@@ -235,14 +226,14 @@ func (am *Manager) ImportECDSA(priv *ecdsa.PrivateKey, keyAuth string) (Account,
 	return Account{Address: key.Address}, nil
 }
 
-func (am *Manager) Update(addr common.Address, authFrom, authTo string) (err error) {
+func (am *Manager) Update(a Account, authFrom, authTo string) (err error) {
 	var key *Key
-	key, err = am.keyStore.GetKey(addr, authFrom)
+	key, err = am.keyStore.GetKey(a.Address, authFrom)
 
 	if err == nil {
 		err = am.keyStore.StoreKey(key, authTo)
 		if err == nil {
-			am.keyStore.Cleanup(addr)
+			am.keyStore.Cleanup(a.Address)
 		}
 	}
 	return

accounts/accounts_test.go

@@ -31,7 +31,7 @@ func TestSign(t *testing.T) {
 
 	pass := "" // not used but required by API
 	a1, err := am.NewAccount(pass)
-	am.Unlock(a1.Address, "")
+	am.Unlock(a1, "")
 
 	_, err = am.Sign(a1, testSigData)
 	if err != nil {
@@ -53,7 +53,7 @@ func TestTimedUnlock(t *testing.T) {
 	}
 
 	// Signing with passphrase works
-	if err = am.TimedUnlock(a1.Address, pass, 100*time.Millisecond); err != nil {
+	if err = am.TimedUnlock(a1, pass, 100*time.Millisecond); err != nil {
 		t.Fatal(err)
 	}
 
@@ -79,7 +79,7 @@ func TestOverrideUnlock(t *testing.T) {
 	a1, err := am.NewAccount(pass)
 
 	// Unlock indefinitely
-	if err = am.Unlock(a1.Address, pass); err != nil {
+	if err = am.Unlock(a1, pass); err != nil {
 		t.Fatal(err)
 	}
 
@@ -90,7 +90,7 @@ func TestOverrideUnlock(t *testing.T) {
 	}
 
 	// reset unlock to a shorter period, invalidates the previous unlock
-	if err = am.TimedUnlock(a1.Address, pass, 100*time.Millisecond); err != nil {
+	if err = am.TimedUnlock(a1, pass, 100*time.Millisecond); err != nil {
 		t.Fatal(err)
 	}
 
@@ -119,7 +119,7 @@ func TestSignRace(t *testing.T) {
 		t.Fatal("could not create the test account", err)
 	}
 
-	if err := am.TimedUnlock(a1.Address, "", 15*time.Millisecond); err != nil {
+	if err := am.TimedUnlock(a1, "", 15*time.Millisecond); err != nil {
 		t.Fatal("could not unlock the test account", err)
 	}
 	end := time.Now().Add(500 * time.Millisecond)

cmd/geth/accountcmd.go

@@ -23,7 +23,6 @@ import (
 	"github.com/codegangsta/cli"
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/cmd/utils"
-	"github.com/ethereum/go-ethereum/common"
 )
 
 var (
@@ -166,17 +165,13 @@ nodes.
 
 func accountList(ctx *cli.Context) {
 	accman := utils.MakeAccountManager(ctx)
-	accts, err := accman.Accounts()
+	for i, acct := range accman.Accounts() {
-	if err != nil {
-		utils.Fatalf("Could not list accounts: %v", err)
-	}
-	for i, acct := range accts {
 		fmt.Printf("Account #%d: %x\n", i, acct)
 	}
 }
 
 // tries unlocking the specified account a few times.
-func unlockAccount(ctx *cli.Context, accman *accounts.Manager, address string, i int, passwords []string) (common.Address, string) {
+func unlockAccount(ctx *cli.Context, accman *accounts.Manager, address string, i int, passwords []string) (accounts.Account, string) {
 	account, err := utils.MakeAddress(accman, address)
 	if err != nil {
 		utils.Fatalf("Could not list accounts: %v", err)
@@ -190,7 +185,7 @@ func unlockAccount(ctx *cli.Context, accman *accounts.Manager, address string, i
 	}
 	// All trials expended to unlock account, bail out
 	utils.Fatalf("Failed to unlock account: %s", address)
-	return common.Address{}, ""
+	return accounts.Account{}, ""
 }
 
 // getPassPhrase retrieves the passwor associated with an account, either fetched

cmd/geth/js.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/codegangsta/cli"
+	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/registrar"
@@ -281,7 +282,8 @@ func (self *jsre) UnlockAccount(addr []byte) bool {
 	if err := self.stack.Service(&ethereum); err != nil {
 		return false
 	}
-	if err := ethereum.AccountManager().Unlock(common.BytesToAddress(addr), pass); err != nil {
+	a := accounts.Account{Address: common.BytesToAddress(addr)}
+	if err := ethereum.AccountManager().Unlock(a, pass); err != nil {
 		return false
 	} else {
 		fmt.Println("Account is now unlocked for this session.")

cmd/geth/js_test.go

@@ -61,17 +61,6 @@ type testjethre struct {
 	client      *httpclient.HTTPClient
 }
 
-func (self *testjethre) UnlockAccount(acc []byte) bool {
-	var ethereum *eth.Ethereum
-	self.stack.Service(&ethereum)
-
-	err := ethereum.AccountManager().Unlock(common.BytesToAddress(acc), "")
-	if err != nil {
-		panic("unable to unlock")
-	}
-	return true
-}
-
 // Temporary disabled while natspec hasn't been migrated
 //func (self *testjethre) ConfirmTransaction(tx string) bool {
 //	var ethereum *eth.Ethereum
@@ -122,7 +111,7 @@ func testREPL(t *testing.T, config func(*eth.Config)) (string, *testjethre, *nod
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := accman.Unlock(a.Address, ""); err != nil {
+	if err := accman.Unlock(a, ""); err != nil {
 		t.Fatal(err)
 	}
 	// Start the node and assemble the REPL tester

cmd/gethrpctest/main.go

@@ -116,7 +116,7 @@ func MakeSystemNode(keydir string, privkey string, test *tests.BlockTest) (*node
 		if err != nil {
 			return nil, err
 		}
-		if err := accman.Unlock(a.Address, ""); err != nil {
+		if err := accman.Unlock(a, ""); err != nil {
 			return nil, err
 		}
 	}

cmd/utils/flags.go

@@ -564,27 +564,23 @@ func MakeAccountManager(ctx *cli.Context) *accounts.Manager {
 
 // MakeAddress converts an account specified directly as a hex encoded string or
 // a key index in the key store to an internal account representation.
-func MakeAddress(accman *accounts.Manager, account string) (a common.Address, err error) {
+func MakeAddress(accman *accounts.Manager, account string) (accounts.Account, error) {
 	// If the specified account is a valid address, return it
 	if common.IsHexAddress(account) {
-		return common.HexToAddress(account), nil
+		return accounts.Account{Address: common.HexToAddress(account)}, nil
 	}
 	// Otherwise try to interpret the account as a keystore index
 	index, err := strconv.Atoi(account)
 	if err != nil {
-		return a, fmt.Errorf("invalid account address or index %q", account)
+		return accounts.Account{}, fmt.Errorf("invalid account address or index %q", account)
 	}
-	hex, err := accman.AddressByIndex(index)
+	return accman.AccountByIndex(index)
-	if err != nil {
-		return a, fmt.Errorf("can't get account #%d (%v)", index, err)
-	}
-	return common.HexToAddress(hex), nil
 }
 
 // MakeEtherbase retrieves the etherbase either from the directly specified
 // command line flags or from the keystore if CLI indexed.
 func MakeEtherbase(accman *accounts.Manager, ctx *cli.Context) common.Address {
-	accounts, _ := accman.Accounts()
+	accounts := accman.Accounts()
 	if !ctx.GlobalIsSet(EtherbaseFlag.Name) && len(accounts) == 0 {
 		glog.V(logger.Error).Infoln("WARNING: No etherbase set and no accounts found as default")
 		return common.Address{}
@@ -594,11 +590,11 @@ func MakeEtherbase(accman *accounts.Manager, ctx *cli.Context) common.Address {
 		return common.Address{}
 	}
 	// If the specified etherbase is a valid address, return it
-	addr, err := MakeAddress(accman, etherbase)
+	account, err := MakeAddress(accman, etherbase)
 	if err != nil {
 		Fatalf("Option %q: %v", EtherbaseFlag.Name, err)
 	}
-	return addr
+	return account.Address
 }
 
 // MakeMinerExtra resolves extradata for the miner from the set command line flags

common/registrar/ethreg/api.go

@@ -158,8 +158,8 @@ func (be *registryAPIBackend) Call(fromStr, toStr, valueStr, gasStr, gasPriceStr
 
 	var from *state.StateObject
 	if len(fromStr) == 0 {
-		accounts, err := be.am.Accounts()
+		accounts := be.am.Accounts()
-		if err != nil || len(accounts) == 0 {
+		if len(accounts) == 0 {
 			from = statedb.GetOrNewStateObject(common.Address{})
 		} else {
 			from = statedb.GetOrNewStateObject(accounts[0].Address)
@@ -254,7 +254,7 @@ func (be *registryAPIBackend) Transact(fromStr, toStr, nonceStr, valueStr, gasSt
 		tx = types.NewTransaction(nonce, to, value, gas, price, data)
 	}
 
-	acc := accounts.Account{from}
+	acc := accounts.Account{Address: from}
 	signature, err := be.am.Sign(acc, tx.SigHash().Bytes())
 	if err != nil {
 		return "", err

eth/api.go

@@ -29,8 +29,6 @@ import (
 	"sync"
 	"time"
 
-	"golang.org/x/net/context"
-
 	"github.com/ethereum/ethash"
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/common"
@@ -48,7 +46,7 @@ import (
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/ethereum/go-ethereum/rpc"
-	"gopkg.in/fatih/set.v0"
+	"golang.org/x/net/context"
 )
 
 const defaultGas = uint64(90000)
@@ -405,7 +403,7 @@ func NewPublicAccountAPI(am *accounts.Manager) *PublicAccountAPI {
 }
 
 // Accounts returns the collection of accounts this node manages
-func (s *PublicAccountAPI) Accounts() ([]accounts.Account, error) {
+func (s *PublicAccountAPI) Accounts() []accounts.Account {
 	return s.am.Accounts()
 }
 
@@ -421,17 +419,13 @@ func NewPrivateAccountAPI(am *accounts.Manager) *PrivateAccountAPI {
 }
 
 // ListAccounts will return a list of addresses for accounts this node manages.
-func (s *PrivateAccountAPI) ListAccounts() ([]common.Address, error) {
+func (s *PrivateAccountAPI) ListAccounts() []common.Address {
-	accounts, err := s.am.Accounts()
+	accounts := s.am.Accounts()
-	if err != nil {
-		return nil, err
-	}
-
 	addresses := make([]common.Address, len(accounts))
 	for i, acc := range accounts {
 		addresses[i] = acc.Address
 	}
-	return addresses, nil
+	return addresses
 }
 
 // NewAccount will create a new account and returns the address for the new account.
@@ -450,8 +444,9 @@ func (s *PrivateAccountAPI) UnlockAccount(addr common.Address, password string,
 	if duration == nil {
 		duration = rpc.NewHexNumber(300)
 	}
-
+	a := accounts.Account{Address: addr}
-	if err := s.am.TimedUnlock(addr, password, time.Duration(duration.Int())*time.Second); err != nil {
+	d := time.Duration(duration.Int64()) * time.Second
+	if err := s.am.TimedUnlock(a, password, d); err != nil {
 		glog.V(logger.Info).Infof("%v\n", err)
 		return false
 	}
@@ -701,8 +696,8 @@ func (s *PublicBlockChainAPI) doCall(args CallArgs, blockNr rpc.BlockNumber) (st
 	// Retrieve the account state object to interact with
 	var from *state.StateObject
 	if args.From == (common.Address{}) {
-		accounts, err := s.am.Accounts()
+		accounts := s.am.Accounts()
-		if err != nil || len(accounts) == 0 {
+		if len(accounts) == 0 {
 			from = stateDb.GetOrNewStateObject(common.Address{})
 		} else {
 			from = stateDb.GetOrNewStateObject(accounts[0].Address)
@@ -912,24 +907,10 @@ func NewPublicTransactionPoolAPI(e *Ethereum, gpo *GasPriceOracle) *PublicTransa
 // subscriptionLoop listens for events on the global event mux and creates notifications for subscriptions.
 func (s *PublicTransactionPoolAPI) subscriptionLoop() {
 	sub := s.eventMux.Subscribe(core.TxPreEvent{})
-	accountTimeout := time.NewTicker(10 * time.Second)
+	for event := range sub.Chan() {
-
-	// only publish pending tx signed by one of the accounts in the node
-	accountSet := set.New()
-	accounts, _ := s.am.Accounts()
-	for _, acc := range accounts {
-		accountSet.Add(acc.Address)
-	}
-
-	for {
-		select {
-		case event := <-sub.Chan():
-			if event == nil {
-				continue
-			}
 		tx := event.Data.(core.TxPreEvent)
 		if from, err := tx.Tx.FromFrontier(); err == nil {
-				if accountSet.Has(from) {
+			if s.am.HasAddress(from) {
 				s.muPendingTxSubs.Lock()
 				for id, sub := range s.pendingTxSubs {
 					if sub.Notify(tx.Tx.Hash()) == rpc.ErrNotificationNotFound {
@@ -939,15 +920,6 @@ func (s *PublicTransactionPoolAPI) subscriptionLoop() {
 				s.muPendingTxSubs.Unlock()
 			}
 		}
-		case <-accountTimeout.C:
-			// refresh account list when accounts are added/removed from the node.
-			if accounts, err := s.am.Accounts(); err == nil {
-				accountSet.Clear()
-				for _, acc := range accounts {
-					accountSet.Add(acc.Address)
-				}
-			}
-		}
 	}
 }
 
@@ -1116,7 +1088,7 @@ func (s *PublicTransactionPoolAPI) GetTransactionReceipt(txHash common.Hash) (ma
 
 // sign is a helper function that signs a transaction with the private key of the given address.
 func (s *PublicTransactionPoolAPI) sign(address common.Address, tx *types.Transaction) (*types.Transaction, error) {
-	acc := accounts.Account{address}
+	acc := accounts.Account{Address: address}
 	signature, err := s.am.Sign(acc, tx.SigHash().Bytes())
 	if err != nil {
 		return nil, err
@@ -1358,26 +1330,16 @@ func (s *PublicTransactionPoolAPI) SignTransaction(args SignTransactionArgs) (*S
 
 // PendingTransactions returns the transactions that are in the transaction pool and have a from address that is one of
 // the accounts this node manages.
-func (s *PublicTransactionPoolAPI) PendingTransactions() ([]*RPCTransaction, error) {
+func (s *PublicTransactionPoolAPI) PendingTransactions() []*RPCTransaction {
-	accounts, err := s.am.Accounts()
-	if err != nil {
-		return nil, err
-	}
-
-	accountSet := set.New()
-	for _, account := range accounts {
-		accountSet.Add(account.Address)
-	}
-
 	pending := s.txPool.GetTransactions()
 	transactions := make([]*RPCTransaction, 0)
 	for _, tx := range pending {
-		if from, _ := tx.FromFrontier(); accountSet.Has(from) {
+		from, _ := tx.FromFrontier()
+		if s.am.HasAddress(from) {
 			transactions = append(transactions, newRPCPendingTransaction(tx))
 		}
 	}
-
+	return transactions
-	return transactions, nil
 }
 
 // NewPendingTransaction creates a subscription that is triggered each time a transaction enters the transaction pool
@@ -1856,8 +1818,8 @@ func (s *PublicBlockChainAPI) TraceCall(args CallArgs, blockNr rpc.BlockNumber)
 	// Retrieve the account state object to interact with
 	var from *state.StateObject
 	if args.From == (common.Address{}) {
-		accounts, err := s.am.Accounts()
+		accounts := s.am.Accounts()
-		if err != nil || len(accounts) == 0 {
+		if len(accounts) == 0 {
 			from = stateDb.GetOrNewStateObject(common.Address{})
 		} else {
 			from = stateDb.GetOrNewStateObject(accounts[0].Address)

eth/backend.go

@@ -359,13 +359,13 @@ func (s *Ethereum) ResetWithGenesisBlock(gb *types.Block) {
 func (s *Ethereum) Etherbase() (eb common.Address, err error) {
 	eb = s.etherbase
 	if (eb == common.Address{}) {
-		addr, e := s.AccountManager().AddressByIndex(0)
+		firstAccount, err := s.AccountManager().AccountByIndex(0)
-		if e != nil {
+		eb = firstAccount.Address
-			err = fmt.Errorf("etherbase address must be explicitly specified")
+		if err != nil {
+			return eb, fmt.Errorf("etherbase address must be explicitly specified")
 		}
-		eb = common.HexToAddress(addr)
 	}
-	return
+	return eb, nil
 }
 
 // set in js console via admin interface or wrapper from cli flags

miner/worker.go

@@ -388,7 +388,7 @@ func (self *worker) makeCurrent(parent *types.Block, header *types.Header) error
 		work.family.Add(ancestor.Hash())
 		work.ancestors.Add(ancestor.Hash())
 	}
-	accounts, _ := self.eth.AccountManager().Accounts()
+	accounts := self.eth.AccountManager().Accounts()
 
 	// Keep track of transactions which return errors so they can be removed
 	work.remove = set.New()