From 46e8940b19fee9bc21767a1341c382fd9c9d572a Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Thu, 3 Mar 2016 01:09:16 +0100
Subject: [PATCH] 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 | 57 +++++++++------------
accounts/accounts_test.go | 10 ++--
cmd/geth/accountcmd.go | 11 ++---
cmd/geth/js.go | 4 +-
cmd/geth/js_test.go | 13 +----
cmd/gethrpctest/main.go | 2 +-
cmd/utils/flags.go | 18 +++----
common/registrar/ethreg/api.go | 6 +--
eth/api.go | 90 ++++++++++------------------------
eth/backend.go | 10 ++--
miner/worker.go | 2 +-
11 files changed, 79 insertions(+), 144 deletions(-)
diff --git a/accounts/account_manager.go b/accounts/account_manager.go
index c85304066..acf4d8e21 100644
--- a/accounts/account_manager.go
+++ b/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 {
- accounts, _ := am.Accounts()
+func (am *Manager) HasAddress(addr common.Address) bool {
+ 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 {
- return am.keyStore.DeleteKey(address, auth)
+func (am *Manager) DeleteAccount(a Account, auth string) error {
+ 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 {
- return am.TimedUnlock(addr, keyAuth, 0)
+func (am *Manager) Unlock(a Account, keyAuth string) error {
+ 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 {
- key, err := am.keyStore.GetKey(addr, keyAuth)
+func (am *Manager) TimedUnlock(a Account, keyAuth string, timeout time.Duration) error {
+ 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) {
- var addrs []common.Address
- addrs, err = am.keyStore.GetKeyAddresses()
+func (am *Manager) AccountByIndex(index int) (Account, error) {
+ 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)
- } else {
- addr = addrs[index].Hex()
+ return Account{}, fmt.Errorf("account index %d not in range [0, %d]", index, len(addrs)-1)
}
- return
+ return Account{Address: addrs[index]}, nil
}
-func (am *Manager) Accounts() ([]Account, error) {
- addresses, err := am.keyStore.GetKeyAddresses()
- if os.IsNotExist(err) {
- return nil, ErrNoKeys
- } else if err != nil {
- return nil, err
- }
+func (am *Manager) Accounts() []Account {
+ addresses, _ := am.keyStore.GetKeyAddresses()
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 {
- key, err := am.keyStore.GetKey(addr, keyAuth)
+func (am *Manager) Export(path string, a Account, keyAuth string) error {
+ 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
diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go
index 4a1d1b848..0cb87a8f1 100644
--- a/accounts/accounts_test.go
+++ b/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)
diff --git a/cmd/geth/accountcmd.go b/cmd/geth/accountcmd.go
index ec6de886f..b4c37cb86 100644
--- a/cmd/geth/accountcmd.go
+++ b/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()
- if err != nil {
- utils.Fatalf("Could not list accounts: %v", err)
- }
- for i, acct := range accts {
+ for i, acct := range accman.Accounts() {
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
diff --git a/cmd/geth/js.go b/cmd/geth/js.go
index 5178465d1..d5518f94b 100644
--- a/cmd/geth/js.go
+++ b/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(ðereum); 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.")
diff --git a/cmd/geth/js_test.go b/cmd/geth/js_test.go
index 77e40bb9a..baf572359 100644
--- a/cmd/geth/js_test.go
+++ b/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(ðereum)
-
- 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
diff --git a/cmd/gethrpctest/main.go b/cmd/gethrpctest/main.go
index e203b75a1..b25166f4f 100644
--- a/cmd/gethrpctest/main.go
+++ b/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
}
}
diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go
index c87c2f76e..da29ceb09 100644
--- a/cmd/utils/flags.go
+++ b/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)
- if err != nil {
- return a, fmt.Errorf("can't get account #%d (%v)", index, err)
- }
- return common.HexToAddress(hex), nil
+ return accman.AccountByIndex(index)
}
// 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
diff --git a/common/registrar/ethreg/api.go b/common/registrar/ethreg/api.go
index d035616f2..4bffe69c5 100644
--- a/common/registrar/ethreg/api.go
+++ b/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()
- if err != nil || len(accounts) == 0 {
+ accounts := be.am.Accounts()
+ 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
diff --git a/eth/api.go b/eth/api.go
index 138df908a..20cf6de39 100644
--- a/eth/api.go
+++ b/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) {
- accounts, err := s.am.Accounts()
- if err != nil {
- return nil, err
- }
-
+func (s *PrivateAccountAPI) ListAccounts() []common.Address {
+ accounts := s.am.Accounts()
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)
}
-
- if err := s.am.TimedUnlock(addr, password, time.Duration(duration.Int())*time.Second); err != nil {
+ a := accounts.Account{Address: addr}
+ 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()
- if err != nil || len(accounts) == 0 {
+ accounts := s.am.Accounts()
+ if len(accounts) == 0 {
from = stateDb.GetOrNewStateObject(common.Address{})
} else {
from = stateDb.GetOrNewStateObject(accounts[0].Address)
@@ -912,40 +907,17 @@ 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)
-
- // 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) {
- s.muPendingTxSubs.Lock()
- for id, sub := range s.pendingTxSubs {
- if sub.Notify(tx.Tx.Hash()) == rpc.ErrNotificationNotFound {
- delete(s.pendingTxSubs, id)
- }
+ for event := range sub.Chan() {
+ tx := event.Data.(core.TxPreEvent)
+ if from, err := tx.Tx.FromFrontier(); err == nil {
+ if s.am.HasAddress(from) {
+ s.muPendingTxSubs.Lock()
+ for id, sub := range s.pendingTxSubs {
+ if sub.Notify(tx.Tx.Hash()) == rpc.ErrNotificationNotFound {
+ delete(s.pendingTxSubs, id)
}
- 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)
}
+ s.muPendingTxSubs.Unlock()
}
}
}
@@ -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) {
- accounts, err := s.am.Accounts()
- if err != nil {
- return nil, err
- }
-
- accountSet := set.New()
- for _, account := range accounts {
- accountSet.Add(account.Address)
- }
-
+func (s *PublicTransactionPoolAPI) PendingTransactions() []*RPCTransaction {
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, nil
+ return transactions
}
// 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()
- if err != nil || len(accounts) == 0 {
+ accounts := s.am.Accounts()
+ if len(accounts) == 0 {
from = stateDb.GetOrNewStateObject(common.Address{})
} else {
from = stateDb.GetOrNewStateObject(accounts[0].Address)
diff --git a/eth/backend.go b/eth/backend.go
index f4282d59f..12ce30767 100644
--- a/eth/backend.go
+++ b/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)
- if e != nil {
- err = fmt.Errorf("etherbase address must be explicitly specified")
+ firstAccount, err := s.AccountManager().AccountByIndex(0)
+ eb = firstAccount.Address
+ 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
diff --git a/miner/worker.go b/miner/worker.go
index c5fb82b45..68e99053f 100644
--- a/miner/worker.go
+++ b/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()
--
GitLab