Maintain local copy of the nonce for each used address (#538)

This commit is contained in:
Dmitry Shulyak 2018-02-08 00:23:57 +02:00 committed by Ivan Daniluk
parent ca719af71c
commit d0ef64a177
6 changed files with 152 additions and 122 deletions

2
Gopkg.lock generated
View File

@ -109,7 +109,7 @@
"whisper/notifications",
"whisper/whisperv5"
]
revision = "f392249eb323a64d188dd53648975b8a5ca25eda"
revision = "6b396471b29c6fdf9f05eb85dd0eaf8544ede3d9"
source = "https://github.com/status-im/go-ethereum.git"
[[projects]]

View File

@ -1,19 +0,0 @@
diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go
index 362379cc..6e12e500 100644
--- a/internal/ethapi/api.go
+++ b/internal/ethapi/api.go
@@ -956,6 +956,14 @@ func (s *PublicTransactionPoolAPI) GetRawTransactionByBlockHashAndIndex(ctx cont
// GetTransactionCount returns the number of transactions the given address has sent for the given block number
func (s *PublicTransactionPoolAPI) GetTransactionCount(ctx context.Context, address common.Address, blockNr rpc.BlockNumber) (*hexutil.Uint64, error) {
+ // go-ethereum issue https://github.com/ethereum/go-ethereum/issues/2880
+ if blockNr == rpc.PendingBlockNumber {
+ nonce, err := s.b.GetPoolNonce(ctx, address)
+ if err != nil {
+ return nil, err
+ }
+ return (*hexutil.Uint64)(&nonce), nil
+ }
state, _, err := s.b.StateAndHeaderByNumber(ctx, blockNr)
if state == nil || err != nil {
return nil, err

View File

@ -15,9 +15,7 @@ We try to minimize number and amount of changes in those patches as much as poss
- [`0004-whisper-notifications.patch`](./0004-whisper-notifications.patch) — adds Whisper notifications (need to be reviewed and documented)
- [`0006-latest-cht.patch`](./0006-latest-cht.patch) updates CHT root hashes, should be updated regularly to keep sync fast, until proper Trusted Checkpoint sync is not implemented as part of LES/2 protocol.
- [`0007-README.patch`](./0007-README.patch) — update upstream README.md.
- [`0008-tx-pool-nonce.patch`](./0008-tx-pool-nonce.patch) - On GetTransactionCount request with PendingBlockNumber get the nonce from transaction pool
- [`0010-geth-17-fix-npe-in-filter-system.patch`](./0010-geth-17-fix-npe-in-filter-system.patch) - Temp patch for 1.7.x to fix a NPE in the filter system
- [`0010-geth-17-fix-npe-in-filter-system.patch`](./0010-geth-17-fix-npe-in-filter-system.patch) - Temp patch for 1.7.x to fix a NPE in the filter system.
# Updating upstream version
When a new stable release of `go-ethereum` comes out, we need to upgrade our fork and vendored copy.

View File

@ -3,6 +3,7 @@ package transactions
import (
"context"
"math/big"
"sync"
"time"
ethereum "github.com/ethereum/go-ethereum"
@ -10,6 +11,7 @@ import (
"github.com/ethereum/go-ethereum/core/types"
"github.com/status-im/status-go/geth/common"
"github.com/status-im/status-go/geth/log"
"github.com/status-im/status-go/geth/params"
"github.com/status-im/status-go/geth/transactions/queue"
)
@ -29,10 +31,12 @@ type Manager struct {
accountManager common.AccountManager
txQueue *queue.TxQueue
ethTxClient EthTransactor
addrLock *AddrLocker
notify bool
completionTimeout time.Duration
rpcCallTimeout time.Duration
addrLock *AddrLocker
localNonce sync.Map
}
// NewManager returns a new Manager.
@ -45,6 +49,7 @@ func NewManager(nodeManager common.NodeManager, accountManager common.AccountMan
notify: true,
completionTimeout: DefaultTxSendCompletionTimeout,
rpcCallTimeout: defaultTimeout,
localNonce: sync.Map{},
}
}
@ -128,19 +133,22 @@ func (m *Manager) CompleteTransaction(id common.QueuedTxID, password string) (ha
log.Warn("can't process transaction", "err", err)
return hash, err
}
account, err := m.validateAccount(tx)
config, err := m.nodeManager.NodeConfig()
if err != nil {
return hash, err
}
account, err := m.validateAccount(config, tx, password)
if err != nil {
m.txDone(tx, hash, err)
return hash, err
}
// Send the transaction finally.
hash, err = m.completeTransaction(tx, account, password)
hash, err = m.completeTransaction(config, account, tx)
log.Info("finally completed transaction", "id", tx.ID, "hash", hash, "err", err)
m.txDone(tx, hash, err)
return hash, err
}
func (m *Manager) validateAccount(tx *common.QueuedTx) (*common.SelectedExtKey, error) {
func (m *Manager) validateAccount(config *params.NodeConfig, tx *common.QueuedTx, password string) (*common.SelectedExtKey, error) {
selectedAccount, err := m.accountManager.SelectedAccount()
if err != nil {
log.Warn("failed to get a selected account", "err", err)
@ -151,29 +159,42 @@ func (m *Manager) validateAccount(tx *common.QueuedTx) (*common.SelectedExtKey,
log.Warn("queued transaction does not belong to the selected account", "err", queue.ErrInvalidCompleteTxSender)
return nil, queue.ErrInvalidCompleteTxSender
}
return selectedAccount, nil
}
func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount *common.SelectedExtKey, password string) (hash gethcommon.Hash, err error) {
log.Info("complete transaction", "id", queuedTx.ID)
log.Info("verifying account password for transaction", "id", queuedTx.ID)
config, err := m.nodeManager.NodeConfig()
if err != nil {
return hash, err
}
_, err = m.accountManager.VerifyAccountPassword(config.KeyStoreDir, selectedAccount.Address.String(), password)
if err != nil {
log.Warn("failed to verify account", "account", selectedAccount.Address.String(), "error", err.Error())
return hash, err
return nil, err
}
return selectedAccount, nil
}
func (m *Manager) completeTransaction(config *params.NodeConfig, selectedAccount *common.SelectedExtKey, queuedTx *common.QueuedTx) (hash gethcommon.Hash, err error) {
log.Info("complete transaction", "id", queuedTx.ID)
m.addrLock.LockAddr(queuedTx.Args.From)
defer m.addrLock.UnlockAddr(queuedTx.Args.From)
var localNonce uint64
if val, ok := m.localNonce.Load(queuedTx.Args.From); ok {
localNonce = val.(uint64)
}
var nonce uint64
defer func() {
// nonce should be incremented only if tx completed without error
// if upstream node returned nonce higher than ours we will stick to it
if err == nil {
m.localNonce.Store(queuedTx.Args.From, nonce+1)
}
m.addrLock.UnlockAddr(queuedTx.Args.From)
}()
ctx, cancel := context.WithTimeout(context.Background(), m.rpcCallTimeout)
defer cancel()
nonce, err := m.ethTxClient.PendingNonceAt(ctx, queuedTx.Args.From)
nonce, err = m.ethTxClient.PendingNonceAt(ctx, queuedTx.Args.From)
if err != nil {
return hash, err
}
// if upstream node returned nonce higher than ours we will use it, as it probably means
// that another client was used for sending transactions
if localNonce > nonce {
nonce = localNonce
}
args := queuedTx.Args
gasPrice := (*big.Int)(args.GasPrice)
if args.GasPrice == nil {

View File

@ -2,6 +2,7 @@ package transactions
import (
"context"
"errors"
"math/big"
"sync"
"testing"
@ -39,6 +40,9 @@ type TxQueueTestSuite struct {
client *gethrpc.Client
txServiceMockCtrl *gomock.Controller
txServiceMock *fake.MockPublicTransactionPoolAPI
nodeConfig *params.NodeConfig
manager *Manager
}
func (s *TxQueueTestSuite) SetupTest() {
@ -53,9 +57,19 @@ func (s *TxQueueTestSuite) SetupTest() {
s.client = gethrpc.DialInProc(s.server)
rpclient, _ := rpc.NewClient(s.client, params.UpstreamRPCConfig{})
s.nodeManagerMock.EXPECT().RPCClient().Return(rpclient)
nodeConfig, err := params.NewNodeConfig("/tmp", params.RopstenNetworkID, true)
s.Require().NoError(err)
s.nodeConfig = nodeConfig
s.manager = NewManager(s.nodeManagerMock, s.accountManagerMock)
s.manager.DisableNotificactions()
s.manager.completionTimeout = time.Second
s.manager.rpcCallTimeout = time.Second
s.manager.Start()
}
func (s *TxQueueTestSuite) TearDownTest() {
s.manager.Stop()
s.nodeManagerMockCtrl.Finish()
s.accountManagerMockCtrl.Finish()
s.txServiceMockCtrl.Finish()
@ -69,11 +83,11 @@ var (
testNonce = hexutil.Uint64(10)
)
func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, txErr error) {
func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, returnNonce, resultNonce hexutil.Uint64, account *common.SelectedExtKey, txErr error) {
// Expect calls to gas functions only if there are no user defined values.
// And also set the expected gas and gas price for RLP encoding the expected tx.
var usedGas, usedGasPrice *big.Int
s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&testNonce, nil)
s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&returnNonce, nil)
if tx.Args.GasPrice == nil {
usedGasPrice = (*big.Int)(testGasPrice)
s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(usedGasPrice, nil)
@ -87,7 +101,7 @@ func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *
usedGas = (*big.Int)(tx.Args.Gas)
}
// Prepare the transaction anD RLP encode it.
data := s.rlpEncodeTx(tx, config, account, &testNonce, usedGas, usedGasPrice)
data := s.rlpEncodeTx(tx, s.nodeConfig, account, &resultNonce, usedGas, usedGasPrice)
// Expect the RLP encoded transaction.
s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), data).Return(gethcommon.Hash{}, txErr)
}
@ -109,13 +123,11 @@ func (s *TxQueueTestSuite) rlpEncodeTx(tx *common.QueuedTx, config *params.NodeC
return hexutil.Bytes(data)
}
func (s *TxQueueTestSuite) setupStatusBackend(account *common.SelectedExtKey, password string, passwordErr error) *params.NodeConfig {
nodeConfig, nodeErr := params.NewNodeConfig("/tmp", params.RopstenNetworkID, true)
s.nodeManagerMock.EXPECT().NodeConfig().Return(nodeConfig, nodeErr)
func (s *TxQueueTestSuite) setupStatusBackend(account *common.SelectedExtKey, password string, passwordErr error) {
s.nodeManagerMock.EXPECT().NodeConfig().Return(s.nodeConfig, nil)
s.accountManagerMock.EXPECT().SelectedAccount().Return(account, nil)
s.accountManagerMock.EXPECT().VerifyAccountPassword(nodeConfig.KeyStoreDir, account.Address.String(), password).Return(
s.accountManagerMock.EXPECT().VerifyAccountPassword(s.nodeConfig.KeyStoreDir, account.Address.String(), password).Return(
nil, passwordErr)
return nodeConfig
}
func (s *TxQueueTestSuite) TestCompleteTransaction() {
@ -125,7 +137,6 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() {
Address: common.FromAddress(TestConfig.Account1.Address),
AccountKey: &keystore.Key{PrivateKey: key},
}
testCases := []struct {
name string
gas *hexutil.Big
@ -151,39 +162,32 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() {
for _, testCase := range testCases {
s.T().Run(testCase.name, func(t *testing.T) {
s.SetupTest()
config := s.setupStatusBackend(account, password, nil)
s.setupStatusBackend(account, password, nil)
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
Gas: testCase.gas,
GasPrice: testCase.gasPrice,
})
s.setupTransactionPoolAPI(tx, config, account, nil)
s.setupTransactionPoolAPI(tx, testNonce, testNonce, account, nil)
txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock)
txQueueManager.completionTimeout = time.Second
txQueueManager.rpcCallTimeout = time.Second
txQueueManager.Start()
defer txQueueManager.Stop()
s.NoError(txQueueManager.QueueTransaction(tx))
s.NoError(s.manager.QueueTransaction(tx))
w := make(chan struct{})
var (
hash gethcommon.Hash
err error
)
go func() {
hash, err = txQueueManager.CompleteTransaction(tx.ID, password)
hash, err = s.manager.CompleteTransaction(tx.ID, password)
s.NoError(err)
close(w)
}()
rst := txQueueManager.WaitForTransaction(tx)
rst := s.manager.WaitForTransaction(tx)
// Check that error is assigned to the transaction.
s.NoError(rst.Error)
// Transaction should be already removed from the queue.
s.False(txQueueManager.TransactionQueue().Has(tx.ID))
s.False(s.manager.TransactionQueue().Has(tx.ID))
s.NoError(WaitClosed(w, time.Second))
s.Equal(hash, rst.Hash)
})
@ -197,23 +201,16 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() {
Address: common.FromAddress(TestConfig.Account1.Address),
AccountKey: &keystore.Key{PrivateKey: key},
}
config := s.setupStatusBackend(account, password, nil)
s.setupStatusBackend(account, password, nil)
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.setupTransactionPoolAPI(tx, config, account, nil)
s.setupTransactionPoolAPI(tx, testNonce, testNonce, account, nil)
txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock)
txQueueManager.completionTimeout = time.Second
txQueueManager.rpcCallTimeout = time.Second
txQueueManager.DisableNotificactions()
txQueueManager.Start()
defer txQueueManager.Stop()
err := txQueueManager.QueueTransaction(tx)
err := s.manager.QueueTransaction(tx)
s.NoError(err)
var (
@ -227,7 +224,7 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() {
wg.Add(1)
go func() {
defer wg.Done()
_, err := txQueueManager.CompleteTransaction(tx.ID, password)
_, err := s.manager.CompleteTransaction(tx.ID, password)
mu.Lock()
defer mu.Unlock()
if err == nil {
@ -240,11 +237,11 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() {
}()
}
rst := txQueueManager.WaitForTransaction(tx)
rst := s.manager.WaitForTransaction(tx)
// Check that error is assigned to the transaction.
s.NoError(rst.Error)
// Transaction should be already removed from the queue.
s.False(txQueueManager.TransactionQueue().Has(tx.ID))
s.False(s.manager.TransactionQueue().Has(tx.ID))
// Wait for all CompleteTransaction calls.
wg.Wait()
@ -253,29 +250,24 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() {
}
func (s *TxQueueTestSuite) TestAccountMismatch() {
s.nodeManagerMock.EXPECT().NodeConfig().Return(s.nodeConfig, nil)
s.accountManagerMock.EXPECT().SelectedAccount().Return(&common.SelectedExtKey{
Address: common.FromAddress(TestConfig.Account2.Address),
}, nil)
txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock)
txQueueManager.DisableNotificactions()
txQueueManager.Start()
defer txQueueManager.Stop()
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.NoError(txQueueManager.QueueTransaction(tx))
s.NoError(s.manager.QueueTransaction(tx))
_, err := txQueueManager.CompleteTransaction(tx.ID, TestConfig.Account1.Password)
_, err := s.manager.CompleteTransaction(tx.ID, TestConfig.Account1.Password)
s.Equal(err, queue.ErrInvalidCompleteTxSender)
// Transaction should stay in the queue as mismatched accounts
// is a recoverable error.
s.True(txQueueManager.TransactionQueue().Has(tx.ID))
s.True(s.manager.TransactionQueue().Has(tx.ID))
}
func (s *TxQueueTestSuite) TestInvalidPassword() {
@ -286,67 +278,113 @@ func (s *TxQueueTestSuite) TestInvalidPassword() {
AccountKey: &keystore.Key{PrivateKey: key},
}
s.setupStatusBackend(account, password, keystore.ErrDecrypt)
txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock)
txQueueManager.DisableNotificactions()
txQueueManager.Start()
defer txQueueManager.Stop()
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.NoError(txQueueManager.QueueTransaction(tx))
_, err := txQueueManager.CompleteTransaction(tx.ID, password)
s.NoError(s.manager.QueueTransaction(tx))
_, err := s.manager.CompleteTransaction(tx.ID, password)
s.Equal(err.Error(), keystore.ErrDecrypt.Error())
// Transaction should stay in the queue as mismatched accounts
// is a recoverable error.
s.True(txQueueManager.TransactionQueue().Has(tx.ID))
s.True(s.manager.TransactionQueue().Has(tx.ID))
}
func (s *TxQueueTestSuite) TestDiscardTransaction() {
txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock)
txQueueManager.DisableNotificactions()
txQueueManager.Start()
defer txQueueManager.Stop()
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.NoError(txQueueManager.QueueTransaction(tx))
s.NoError(s.manager.QueueTransaction(tx))
w := make(chan struct{})
go func() {
s.NoError(txQueueManager.DiscardTransaction(tx.ID))
s.NoError(s.manager.DiscardTransaction(tx.ID))
close(w)
}()
rst := txQueueManager.WaitForTransaction(tx)
rst := s.manager.WaitForTransaction(tx)
s.Equal(ErrQueuedTxDiscarded, rst.Error)
// Transaction should be already removed from the queue.
s.False(txQueueManager.TransactionQueue().Has(tx.ID))
s.False(s.manager.TransactionQueue().Has(tx.ID))
s.NoError(WaitClosed(w, time.Second))
}
func (s *TxQueueTestSuite) TestCompletionTimedOut() {
txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock)
txQueueManager.DisableNotificactions()
txQueueManager.completionTimeout = time.Nanosecond
txQueueManager.Start()
defer txQueueManager.Stop()
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.NoError(txQueueManager.QueueTransaction(tx))
rst := txQueueManager.WaitForTransaction(tx)
s.NoError(s.manager.QueueTransaction(tx))
rst := s.manager.WaitForTransaction(tx)
s.Equal(ErrQueuedTxTimedOut, rst.Error)
}
// TestLocalNonce verifies that local nonce will be used unless
// upstream nonce is updated and higher than a local
// in test we will run 3 transaction with nonce zero returned by upstream
// node, after each call local nonce will be incremented
// then, we return higher nonce, as if another node was used to send 2 transactions
// upstream nonce will be equal to 5, we update our local counter to 5+1
// as the last step, we verify that if tx failed nonce is not updated
func (s *TxQueueTestSuite) TestLocalNonce() {
txCount := 3
password := TestConfig.Account1.Password
key, _ := crypto.GenerateKey()
account := &common.SelectedExtKey{
Address: common.FromAddress(TestConfig.Account1.Address),
AccountKey: &keystore.Key{PrivateKey: key},
}
// setup call expectations for 5 transactions in total
for i := 0; i < txCount+2; i++ {
s.setupStatusBackend(account, password, nil)
}
nonce := hexutil.Uint64(0)
for i := 0; i < txCount; i++ {
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.setupTransactionPoolAPI(tx, nonce, hexutil.Uint64(i), account, nil)
s.NoError(s.manager.QueueTransaction(tx))
hash, err := s.manager.CompleteTransaction(tx.ID, password)
rst := s.manager.WaitForTransaction(tx)
// simple sanity checks
s.NoError(err)
s.NoError(rst.Error)
s.Equal(rst.Hash, hash)
resultNonce, _ := s.manager.localNonce.Load(tx.Args.From)
s.Equal(uint64(i)+1, resultNonce.(uint64))
}
nonce = hexutil.Uint64(5)
tx := common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.setupTransactionPoolAPI(tx, nonce, nonce, account, nil)
s.NoError(s.manager.QueueTransaction(tx))
hash, err := s.manager.CompleteTransaction(tx.ID, password)
rst := s.manager.WaitForTransaction(tx)
s.NoError(err)
s.NoError(rst.Error)
s.Equal(rst.Hash, hash)
resultNonce, _ := s.manager.localNonce.Load(tx.Args.From)
s.Equal(uint64(nonce)+1, resultNonce.(uint64))
testErr := errors.New("test")
s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(nil, testErr)
tx = common.CreateTransaction(context.Background(), common.SendTxArgs{
From: common.FromAddress(TestConfig.Account1.Address),
To: common.ToAddress(TestConfig.Account2.Address),
})
s.NoError(s.manager.QueueTransaction(tx))
_, err = s.manager.CompleteTransaction(tx.ID, password)
rst = s.manager.WaitForTransaction(tx)
s.EqualError(testErr, err.Error())
s.EqualError(testErr, rst.Error.Error())
resultNonce, _ = s.manager.localNonce.Load(tx.Args.From)
s.Equal(uint64(nonce)+1, resultNonce.(uint64))
}

View File

@ -976,14 +976,6 @@ func (s *PublicTransactionPoolAPI) GetRawTransactionByBlockHashAndIndex(ctx cont
// GetTransactionCount returns the number of transactions the given address has sent for the given block number
func (s *PublicTransactionPoolAPI) GetTransactionCount(ctx context.Context, address common.Address, blockNr rpc.BlockNumber) (*hexutil.Uint64, error) {
// go-ethereum issue https://github.com/ethereum/go-ethereum/issues/2880
if blockNr == rpc.PendingBlockNumber {
nonce, err := s.b.GetPoolNonce(ctx, address)
if err != nil {
return nil, err
}
return (*hexutil.Uint64)(&nonce), nil
}
state, _, err := s.b.StateAndHeaderByNumber(ctx, blockNr)
if state == nil || err != nil {
return nil, err