From ea11f7dd7aa77856a04d83d0db8f303d02e0ce14 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 25 May 2017 17:08:13 +0200 Subject: [PATCH] internal/ethapi: add mutex around signing + nonce assignment This prevents concurrent assignment of identical nonces when automatic assignment is used. --- eth/bind.go | 2 +- internal/ethapi/addrlock.go | 53 ++++++++++++++++++++++++++++ internal/ethapi/api.go | 70 +++++++++++++++++++------------------ internal/ethapi/backend.go | 5 +-- 4 files changed, 93 insertions(+), 37 deletions(-) create mode 100644 internal/ethapi/addrlock.go diff --git a/eth/bind.go b/eth/bind.go index 2459341834..e5abd86178 100644 --- a/eth/bind.go +++ b/eth/bind.go @@ -48,7 +48,7 @@ func NewContractBackend(apiBackend ethapi.Backend) *ContractBackend { return &ContractBackend{ eapi: ethapi.NewPublicEthereumAPI(apiBackend), bcapi: ethapi.NewPublicBlockChainAPI(apiBackend), - txapi: ethapi.NewPublicTransactionPoolAPI(apiBackend), + txapi: ethapi.NewPublicTransactionPoolAPI(apiBackend, new(ethapi.AddrLocker)), } } diff --git a/internal/ethapi/addrlock.go b/internal/ethapi/addrlock.go new file mode 100644 index 0000000000..5a9c948b83 --- /dev/null +++ b/internal/ethapi/addrlock.go @@ -0,0 +1,53 @@ +// Copyright 2015 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package ethapi + +import ( + "sync" + + "github.com/ethereum/go-ethereum/common" +) + +type AddrLocker struct { + mu sync.Mutex + locks map[common.Address]*sync.Mutex +} + +// lock returns the lock of the given address. +func (l *AddrLocker) lock(address common.Address) *sync.Mutex { + l.mu.Lock() + defer l.mu.Unlock() + if l.locks == nil { + l.locks = make(map[common.Address]*sync.Mutex) + } + if _, ok := l.locks[address]; !ok { + l.locks[address] = new(sync.Mutex) + } + return l.locks[address] +} + +// LockAddr locks an account's mutex. This is used to prevent another tx getting the +// same nonce until the lock is released. The mutex prevents the (an identical nonce) from +// being read again during the time that the first transaction is being signed. +func (l *AddrLocker) LockAddr(address common.Address) { + l.lock(address).Lock() +} + +// UnlockAddr unlocks the mutex of the given account. +func (l *AddrLocker) UnlockAddr(address common.Address) { + l.lock(address).Unlock() +} diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a22c15ecac..371786e4a8 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -23,7 +23,6 @@ import ( "fmt" "math/big" "strings" - "sync" "time" "github.com/ethereum/go-ethereum/accounts" @@ -204,12 +203,13 @@ func (s *PublicAccountAPI) Accounts() []common.Address { // It offers methods to create, (un)lock en list accounts. Some methods accept // passwords and are therefore considered private by default. type PrivateAccountAPI struct { - am *accounts.Manager - b Backend + am *accounts.Manager + nonceLock *AddrLocker + b Backend } // NewPrivateAccountAPI create a new PrivateAccountAPI. -func NewPrivateAccountAPI(b Backend) *PrivateAccountAPI { +func NewPrivateAccountAPI(b Backend, nonceLock *AddrLocker) *PrivateAccountAPI { return &PrivateAccountAPI{ am: b.AccountManager(), b: b, @@ -316,10 +316,6 @@ func (s *PrivateAccountAPI) LockAccount(addr common.Address) bool { // tries to sign it with the key associated with args.To. If the given passwd isn't // able to decrypt the key it fails. func (s *PrivateAccountAPI) SendTransaction(ctx context.Context, args SendTxArgs, passwd string) (common.Hash, error) { - // Set some sanity defaults and terminate on failure - if err := args.setDefaults(ctx, s.b); err != nil { - return common.Hash{}, err - } // Look up the wallet containing the requested signer account := accounts.Account{Address: args.From} @@ -327,6 +323,18 @@ func (s *PrivateAccountAPI) SendTransaction(ctx context.Context, args SendTxArgs if err != nil { return common.Hash{}, err } + + if args.Nonce == nil { + // Hold the addresse's mutex around signing to prevent concurrent assignment of + // the same nonce to multiple accounts. + s.nonceLock.LockAddr(args.From) + defer s.nonceLock.UnlockAddr(args.From) + } + + // Set some sanity defaults and terminate on failure + if err := args.setDefaults(ctx, s.b); err != nil { + return common.Hash{}, err + } // Assemble the transaction and sign with the wallet tx := args.toTransaction() @@ -886,18 +894,13 @@ func newRPCTransaction(b *types.Block, txHash common.Hash) (*RPCTransaction, err // PublicTransactionPoolAPI exposes methods for the RPC interface type PublicTransactionPoolAPI struct { - b Backend + b Backend + nonceLock *AddrLocker } -// nonceMutex is a global mutex for locking the nonce while a transaction -// is being submitted. This should be used when a nonce has not been provided by the user, -// and we get a nonce from the pools. The mutex prevents the (an identical nonce) from being -// read again during the time that the first transaction is being signed. -var nonceMutex sync.RWMutex - // NewPublicTransactionPoolAPI creates a new RPC service with methods specific for the transaction pool. -func NewPublicTransactionPoolAPI(b Backend) *PublicTransactionPoolAPI { - return &PublicTransactionPoolAPI{b} +func NewPublicTransactionPoolAPI(b Backend, nonceLock *AddrLocker) *PublicTransactionPoolAPI { + return &PublicTransactionPoolAPI{b, nonceLock} } func getTransaction(chainDb ethdb.Database, b Backend, txHash common.Hash) (*types.Transaction, bool, error) { @@ -1176,17 +1179,6 @@ func submitTransaction(ctx context.Context, b Backend, tx *types.Transaction) (c // transaction pool. func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args SendTxArgs) (common.Hash, error) { - if args.Nonce == nil { - // We'll need to set nonce from pool, and thus we need to lock here - nonceMutex.Lock() - defer nonceMutex.Unlock() - - } - - // Set some sanity defaults and terminate on failure - if err := args.setDefaults(ctx, s.b); err != nil { - return common.Hash{}, err - } // Look up the wallet containing the requested signer account := accounts.Account{Address: args.From} @@ -1194,6 +1186,18 @@ func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args Sen if err != nil { return common.Hash{}, err } + + if args.Nonce == nil { + // Hold the addresse's mutex around signing to prevent concurrent assignment of + // the same nonce to multiple accounts. + s.nonceLock.LockAddr(args.From) + defer s.nonceLock.UnlockAddr(args.From) + } + + // Set some sanity defaults and terminate on failure + if err := args.setDefaults(ctx, s.b); err != nil { + return common.Hash{}, err + } // Assemble the transaction and sign with the wallet tx := args.toTransaction() @@ -1270,14 +1274,12 @@ type SignTransactionResult struct { // The node needs to have the private key of the account corresponding with // the given from address and it needs to be unlocked. func (s *PublicTransactionPoolAPI) SignTransaction(ctx context.Context, args SendTxArgs) (*SignTransactionResult, error) { - if args.Nonce == nil { - // We'll need to set nonce from pool, and thus we need to lock here - nonceMutex.Lock() - defer nonceMutex.Unlock() - + // Hold the addresse's mutex around signing to prevent concurrent assignment of + // the same nonce to multiple accounts. + s.nonceLock.LockAddr(args.From) + defer s.nonceLock.UnlockAddr(args.From) } - if err := args.setDefaults(ctx, s.b); err != nil { return nil, err } diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go index 42bf26613b..68b5069d08 100644 --- a/internal/ethapi/backend.go +++ b/internal/ethapi/backend.go @@ -73,6 +73,7 @@ type State interface { } func GetAPIs(apiBackend Backend) []rpc.API { + nonceLock := new(AddrLocker) return []rpc.API{ { Namespace: "eth", @@ -87,7 +88,7 @@ func GetAPIs(apiBackend Backend) []rpc.API { }, { Namespace: "eth", Version: "1.0", - Service: NewPublicTransactionPoolAPI(apiBackend), + Service: NewPublicTransactionPoolAPI(apiBackend, nonceLock), Public: true, }, { Namespace: "txpool", @@ -111,7 +112,7 @@ func GetAPIs(apiBackend Backend) []rpc.API { }, { Namespace: "personal", Version: "1.0", - Service: NewPrivateAccountAPI(apiBackend), + Service: NewPrivateAccountAPI(apiBackend, nonceLock), Public: false, }, }