Address review comments

e2e/transactions/transactions_test.go

@@ -790,9 +790,6 @@ func (s *TransactionsTestSuite) TestEvictionOfQueuedTransactions() {
 		go s.Backend.SendTransaction(context.TODO(), common.SendTxArgs{}) // nolint: errcheck
 	}
 	time.Sleep(2 * time.Second)
-
-	log.Info(fmt.Sprintf("Number of transactions sent: %d. Queue size (shouldn't be more than %d): %d",
-		txCount, queue.DefaultTxQueueCap, txQueue.Count()))
 	s.Equal(10, txQueue.Count(), "transaction count should be 10")
 
 	for i := 0; i < queue.DefaultTxQueueCap+5; i++ { // stress test by hitting with lots of goroutines

geth/common/utils.go

@@ -160,6 +160,6 @@ func CreateTransaction(ctx context.Context, args SendTxArgs) *QueuedTx {
 		Hash:    common.Hash{},
 		Context: ctx,
 		Args:    args,
-		Done:    make(chan struct{}, 1),
+		Done:    make(chan struct{}),
 	}
 }

geth/transactions/notifications.go

@@ -1,9 +1,10 @@
 package transactions
 
 import (
+	"strconv"
+
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/status-im/status-go/geth/common"
-	"github.com/status-im/status-go/geth/log"
 	"github.com/status-im/status-go/geth/signal"
 	"github.com/status-im/status-go/geth/transactions/queue"
 )
@@ -13,15 +14,17 @@ const (
 	EventTransactionQueued = "transaction.queued"
 	// EventTransactionFailed is triggered when send transaction request fails
 	EventTransactionFailed = "transaction.failed"
+)
 
-	SendTransactionNoErrorCode        = "0"
-	SendTransactionDefaultErrorCode   = "1"
-	SendTransactionPasswordErrorCode  = "2"
-	SendTransactionTimeoutErrorCode   = "3"
-	SendTransactionDiscardedErrorCode = "4"
+const (
+	SendTransactionNoErrorCode = iota
+	SendTransactionDefaultErrorCode
+	SendTransactionPasswordErrorCode
+	SendTransactionTimeoutErrorCode
+	SendTransactionDiscardedErrorCode
 )
 
-var txReturnCodes = map[error]string{ // deliberately strings, in case more meaningful codes are to be returned
+var txReturnCodes = map[error]int{
 	nil:                        SendTransactionNoErrorCode,
 	keystore.ErrDecrypt:        SendTransactionPasswordErrorCode,
 	queue.ErrQueuedTxTimedOut:  SendTransactionTimeoutErrorCode,
@@ -37,7 +40,6 @@ type SendTransactionEvent struct {
 
 // NotifyOnEnqueue returns handler that processes incoming tx queue requests
 func NotifyOnEnqueue(queuedTx *common.QueuedTx) {
-	log.Info("calling TransactionQueueHandler")
 	signal.Send(signal.Envelope{
 		Type: EventTransactionQueued,
 		Event: SendTransactionEvent{
@@ -59,29 +61,27 @@ type ReturnSendTransactionEvent struct {
 
 // NotifyOnReturn returns handler that processes responses from internal tx manager
 func NotifyOnReturn(queuedTx *common.QueuedTx) {
-	if queuedTx.Err == nil {
-		return
-	}
-
 	// discard notifications with empty tx
 	if queuedTx == nil {
 		return
 	}
-
-	// error occurred, signal up to application
+	// we don't want to notify a user if tx sent successfully
+	if queuedTx.Err == nil {
+		return
+	}
 	signal.Send(signal.Envelope{
 		Type: EventTransactionFailed,
 		Event: ReturnSendTransactionEvent{
 			ID:           string(queuedTx.ID),
 			Args:         queuedTx.Args,
 			MessageID:    common.MessageIDFromContext(queuedTx.Context),
 			ErrorMessage: queuedTx.Err.Error(),
-			ErrorCode:    sendTransactionErrorCode(queuedTx.Err),
+			ErrorCode:    strconv.Itoa(sendTransactionErrorCode(queuedTx.Err)),
 		},
 	})
 }
 
-func sendTransactionErrorCode(err error) string {
+func sendTransactionErrorCode(err error) int {
 	if code, ok := txReturnCodes[err]; ok {
 		return code
 	}

geth/transactions/queue/queue.go

@@ -59,7 +59,7 @@ type TxQueue struct {
 }
 
 // NewTransactionQueue make new transaction queue
-func NewQueue() *TxQueue {
+func New() *TxQueue {
 	log.Info("initializing transaction queue")
 	return &TxQueue{
 		transactions:  make(map[common.QueuedTxID]*common.QueuedTx),
@@ -163,7 +163,7 @@ func (q *TxQueue) Get(id common.QueuedTxID) (*common.QueuedTx, error) {
 	return nil, ErrQueuedTxIDNotFound
 }
 
-// LockInprogress returns transcation and locks it as inprogress
+// LockInprogress returns transation and locks it as inprogress
 func (q *TxQueue) LockInprogress(id common.QueuedTxID) (*common.QueuedTx, error) {
 	q.mu.Lock()
 	defer q.mu.Unlock()
@@ -210,13 +210,12 @@ func (q *TxQueue) done(tx *common.QueuedTx, hash gethcommon.Hash, err error) {
 	if err == nil {
 		q.remove(tx.ID)
 		tx.Hash = hash
-		tx.Done <- struct{}{}
+		close(tx.Done)
 		return
 	}
-	_, transient := transientErrs[err.Error()]
-	if !transient {
+	if _, transient := transientErrs[err.Error()]; !transient {
 		q.remove(tx.ID)
-		tx.Done <- struct{}{}
+		close(tx.Done)
 	}
 }
 

geth/transactions/queue/queue_test.go

@@ -22,7 +22,7 @@ type QueueTestSuite struct {
 }
 
 func (s *QueueTestSuite) SetupTest() {
-	s.queue = NewQueue()
+	s.queue = New()
 	s.queue.Start()
 }
 
@@ -57,6 +57,10 @@ func (s *QueueTestSuite) TestEnqueueProcessedTransaction() {
 	tx := common.CreateTransaction(context.Background(), common.SendTxArgs{})
 	tx.Hash = gethcommon.Hash{1}
 	s.Equal(ErrQueuedTxAlreadyProcessed, s.queue.Enqueue(tx))
+
+	tx = common.CreateTransaction(context.Background(), common.SendTxArgs{})
+	tx.Err = errors.New("error")
+	s.Equal(ErrQueuedTxAlreadyProcessed, s.queue.Enqueue(tx))
 }
 
 func (s *QueueTestSuite) testDone(hash gethcommon.Hash, err error) *common.QueuedTx {
@@ -85,7 +89,6 @@ func (s *QueueTestSuite) TestDoneTransientError() {
 	err := keystore.ErrDecrypt
 	tx := s.testDone(hash, err)
 	s.Equal(keystore.ErrDecrypt, tx.Err)
-	s.NotEqual(hash, tx.Hash)
 	s.Equal(gethcommon.Hash{}, tx.Hash)
 	s.True(s.queue.Has(tx.ID))
 }

geth/transactions/txqueue_manager.go

@@ -38,7 +38,7 @@ func NewManager(nodeManager common.NodeManager, accountManager common.AccountMan
 	return &Manager{
 		nodeManager:    nodeManager,
 		accountManager: accountManager,
-		txQueue:        queue.NewQueue(),
+		txQueue:        queue.New(),
 		addrLock:       &AddrLocker{},
 		notify:         true,
 	}

lib/utils.go

@@ -1056,7 +1056,7 @@ func testDiscardTransaction(t *testing.T) bool { //nolint: gocyclo
 			}
 
 			receivedErrCode := event["error_code"].(string)
-			if receivedErrCode != transactions.SendTransactionDiscardedErrorCode {
+			if receivedErrCode != strconv.Itoa(transactions.SendTransactionDiscardedErrorCode) {
 				t.Errorf("unexpected error code received: got %v", receivedErrCode)
 				return
 			}
@@ -1143,7 +1143,7 @@ func testDiscardMultipleQueuedTransactions(t *testing.T) bool { //nolint: gocycl
 			}
 
 			receivedErrCode := event["error_code"].(string)
-			if receivedErrCode != transactions.SendTransactionDiscardedErrorCode {
+			if receivedErrCode != strconv.Itoa(transactions.SendTransactionDiscardedErrorCode) {
 				t.Errorf("unexpected error code received: got %v", receivedErrCode)
 				return
 			}