From 9c6421dfbfeab0a0de819eee5badeb2685fbdad6 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <yashulyak@gmail.com>
Date: Wed, 24 Jan 2018 13:33:30 +0200
Subject: [PATCH] Rework lock inprogress method

---
 geth/transactions/queue/queue.go      | 13 ++++++-------
 geth/transactions/queue/queue_test.go |  6 +++---
 geth/transactions/txqueue_manager.go  |  8 +++++---
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/geth/transactions/queue/queue.go b/geth/transactions/queue/queue.go
index b1543fd76b5..eea69544263 100644
--- a/geth/transactions/queue/queue.go
+++ b/geth/transactions/queue/queue.go
@@ -163,19 +163,18 @@ func (q *TxQueue) Get(id common.QueuedTxID) (*common.QueuedTx, error) {
 	return nil, ErrQueuedTxIDNotFound
 }
 
-// LockInprogress returns transaction and locks it as inprogress
-func (q *TxQueue) LockInprogress(id common.QueuedTxID) (*common.QueuedTx, error) {
+// LockInprogress returns error if transaction is already inprogress.
+func (q *TxQueue) LockInprogress(id common.QueuedTxID) error {
 	q.mu.Lock()
 	defer q.mu.Unlock()
-
-	if tx, ok := q.transactions[id]; ok {
+	if _, ok := q.transactions[id]; ok {
 		if _, inprogress := q.inprogress[id]; inprogress {
-			return tx, ErrQueuedTxInProgress
+			return ErrQueuedTxInProgress
 		}
 		q.inprogress[id] = empty{}
-		return tx, nil
+		return nil
 	}
-	return nil, ErrQueuedTxIDNotFound
+	return ErrQueuedTxIDNotFound
 }
 
 // Remove removes transaction by transaction identifier
diff --git a/geth/transactions/queue/queue_test.go b/geth/transactions/queue/queue_test.go
index 60abd89d1c2..e156e6b9129 100644
--- a/geth/transactions/queue/queue_test.go
+++ b/geth/transactions/queue/queue_test.go
@@ -33,13 +33,13 @@ func (s *QueueTestSuite) TearDownTest() {
 func (s *QueueTestSuite) TestLockInprogressTransaction() {
 	tx := common.CreateTransaction(context.Background(), common.SendTxArgs{})
 	s.NoError(s.queue.Enqueue(tx))
-	enquedTx, err := s.queue.LockInprogress(tx.ID)
+	enquedTx, err := s.queue.Get(tx.ID)
 	s.NoError(err)
+	s.NoError(s.queue.LockInprogress(tx.ID))
 	s.Equal(tx, enquedTx)
 
 	// verify that tx was marked as being inprogress
-	_, err = s.queue.LockInprogress(tx.ID)
-	s.Equal(ErrQueuedTxInProgress, err)
+	s.Equal(ErrQueuedTxInProgress, s.queue.LockInprogress(tx.ID))
 }
 
 func (s *QueueTestSuite) TestGetTransaction() {
diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go
index a1a9b1ebaeb..bb1ade9c450 100644
--- a/geth/transactions/txqueue_manager.go
+++ b/geth/transactions/txqueue_manager.go
@@ -106,15 +106,17 @@ func (m *Manager) WaitForTransaction(tx *common.QueuedTx) error {
 }
 
 // CompleteTransaction instructs backend to complete sending of a given transaction.
-// TODO(adam): investigate a possible bug that calling this method multiple times with the same Transaction ID
-// results in sending multiple transactions.
 func (m *Manager) CompleteTransaction(id common.QueuedTxID, password string) (hash gethcommon.Hash, err error) {
 	log.Info("complete transaction", "id", id)
-	tx, err := m.txQueue.LockInprogress(id)
+	tx, err := m.txQueue.Get(id)
 	if err != nil {
 		log.Warn("error getting a queued transaction", "err", err)
 		return hash, err
 	}
+	if err := m.txQueue.LockInprogress(id); err != nil {
+		log.Warn("can't process transaction", "err", err)
+		return hash, err
+	}
 	account, err := m.validateAccount(tx)
 	if err != nil {
 		m.txDone(tx, hash, err)