Skip to content

Commit

Permalink
Maintain local copy of the nonce for each used address
Browse files Browse the repository at this point in the history
If multiple concurrent transactions are sent from the same address
nonce for some of them will be the same. It will result in the
"known transaction error" and such transaction will be dropped
from a queue.

There is 2 alternatives in solving such problem:
- adding an auto-retry to transaction queue, which is very complex
  and error prone, this way we can actually retry transactions
  that are already processed by ethereum
- maintain nonce locally

Second approach is a straightforward. We keep asking for a nonce
from upstream, but if our local nonce is higher we steak to it.
Our local nonce is updated only if transaction succeeds, so there is
no way to send out of order transaction.

Signed-off-by: Dmitry Shulyak <yashulyak@gmail.com>
  • Loading branch information
dshulyak committed Feb 6, 2018
1 parent c00e5c9 commit 2eb9d3a
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 120 deletions.
19 changes: 0 additions & 19 deletions geth-patches/0008-tx-pool-nonce.patch

This file was deleted.

4 changes: 1 addition & 3 deletions geth-patches/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
# Updating upstream version

When a new stable release of `go-ethereum` comes out, we need to upgrade our fork and vendored copy.
Expand Down
57 changes: 39 additions & 18 deletions geth/transactions/txqueue_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package transactions
import (
"context"
"math/big"
"sync"
"time"

ethereum "github.com/ethereum/go-ethereum"
gethcommon "github.com/ethereum/go-ethereum/common"
"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"
)

Expand All @@ -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.
Expand All @@ -45,6 +49,7 @@ func NewManager(nodeManager common.NodeManager, accountManager common.AccountMan
notify: true,
completionTimeout: DefaultTxSendCompletionTimeout,
rpcCallTimeout: defaultTimeout,
localNonce: sync.Map{},
}
}

Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
Loading

0 comments on commit 2eb9d3a

Please sign in to comment.