Refactor, fix logging.

txcache/diagnosis.go

@@ -1,10 +1,26 @@
 package txcache
 
 import (
+	"encoding/hex"
+	"encoding/json"
+	"fmt"
+	"strings"
+
 	"github.com/multiversx/mx-chain-core-go/core"
 	logger "github.com/multiversx/mx-chain-logger-go"
 )
 
+type printedTransaction struct {
+	Hash       string  `json:"hash"`
+	Nonce      uint64  `json:"nonce"`
+	PPU        float64 `json:"ppu"`
+	GasPrice   uint64  `json:"gasPrice"`
+	GasLimit   uint64  `json:"gasLimit"`
+	Sender     string  `json:"sender"`
+	Receiver   string  `json:"receiver"`
+	DataLength int     `json:"dataLength"`
+}
+
 // Diagnose checks the state of the cache for inconsistencies and displays a summary, senders and transactions.
 func (cache *TxCache) Diagnose(_ bool) {
 	cache.diagnoseCounters()
@@ -48,37 +64,66 @@ func (cache *TxCache) diagnoseTransactions() {
 	}
 
 	transactions := cache.getAllTransactions()
-
 	if len(transactions) == 0 {
 		return
 	}
 
 	numToDisplay := core.MinInt(diagnosisMaxTransactionsToDisplay, len(transactions))
 	logDiagnoseTransactions.Trace("diagnoseTransactions", "numTransactions", len(transactions), "numToDisplay", numToDisplay)
-	logDiagnoseTransactions.Trace(marshalTransactionsToNewlineDelimitedJson(transactions[:numToDisplay]))
+	logDiagnoseTransactions.Trace(marshalTransactionsToNewlineDelimitedJson(transactions[:numToDisplay], "diagnoseTransactions"))
+}
+
+// marshalTransactionsToNewlineDelimitedJson converts a list of transactions to a newline-delimited JSON string.
+// Note: each line is indexed, to improve readability. The index is easily removable for if separate analysis is needed.
+func marshalTransactionsToNewlineDelimitedJson(transactions []*WrappedTransaction, linePrefix string) string {
+	builder := strings.Builder{}
+	builder.WriteString("\n")
+
+	for i, wrappedTx := range transactions {
+		printedTx := convertWrappedTransactionToPrintedTransaction(wrappedTx)
+		printedTxJson, _ := json.Marshal(printedTx)
+
+		builder.WriteString(fmt.Sprintf("%s#%d: ", linePrefix, i))
+		builder.WriteString(string(printedTxJson))
+		builder.WriteString("\n")
+	}
+
+	builder.WriteString("\n")
+	return builder.String()
+}
+
+func convertWrappedTransactionToPrintedTransaction(wrappedTx *WrappedTransaction) *printedTransaction {
+	transaction := wrappedTx.Tx
+
+	return &printedTransaction{
+		Hash:       hex.EncodeToString(wrappedTx.TxHash),
+		Nonce:      transaction.GetNonce(),
+		Receiver:   hex.EncodeToString(transaction.GetRcvAddr()),
+		Sender:     hex.EncodeToString(transaction.GetSndAddr()),
+		GasPrice:   transaction.GetGasPrice(),
+		GasLimit:   transaction.GetGasLimit(),
+		DataLength: len(transaction.GetData()),
+		PPU:        wrappedTx.PricePerUnit,
+	}
 }
 
 func (cache *TxCache) diagnoseSelection() {
 	if logDiagnoseSelection.GetLevel() > logger.LogDebug {
 		return
 	}
 
-	transactions := cache.doSelectTransactions(
-		logDiagnoseSelection,
-		diagnosisSelectionGasRequested,
-	)
-
-	displaySelectionOutcome(logDiagnoseSelection, transactions)
+	transactions := cache.doSelectTransactions(diagnosisSelectionGasRequested)
+	displaySelectionOutcome(logDiagnoseSelection, "diagnoseSelection", transactions)
 }
 
-func displaySelectionOutcome(contextualLogger logger.Logger, selection []*WrappedTransaction) {
+func displaySelectionOutcome(contextualLogger logger.Logger, linePrefix string, transactions []*WrappedTransaction) {
 	if contextualLogger.GetLevel() > logger.LogTrace {
 		return
 	}
 
-	if len(selection) > 0 {
+	if len(transactions) > 0 {
 		contextualLogger.Trace("displaySelectionOutcome - transactions (as newline-separated JSON):")
-		contextualLogger.Trace(marshalTransactionsToNewlineDelimitedJson(selection))
+		contextualLogger.Trace(marshalTransactionsToNewlineDelimitedJson(transactions, linePrefix))
 	} else {
 		contextualLogger.Trace("displaySelectionOutcome - transactions: none")
 	}

txcache/txCache.go

@@ -66,7 +66,7 @@ func (cache *TxCache) AddTx(tx *WrappedTransaction) (ok bool, added bool) {
 
 	cache.mutTxOperation.Lock()
 	addedInByHash := cache.txByHash.addTx(tx)
-	addedInBySender, evicted := cache.txListBySender.addTx(tx)
+	addedInBySender, evicted := cache.txListBySender.addTxReturnEvicted(tx)
 	cache.mutTxOperation.Unlock()
 	if addedInByHash != addedInBySender {
 		// This can happen  when two go-routines concur to add the same transaction:
@@ -117,7 +117,7 @@ func (cache *TxCache) SelectTransactions(gasRequested uint64) []*WrappedTransact
 	)
 
 	go cache.diagnoseCounters()
-	go displaySelectionOutcome(logSelect, transactions)
+	go displaySelectionOutcome(logSelect, "selection", transactions)
 
 	return transactions
 }

txcache/txCache_test.go

@@ -619,7 +619,7 @@ func TestTxCache_TransactionIsAdded_EvenWhenInternalMapsAreInconsistent(t *testi
 	cache.Clear()
 
 	// Setup inconsistency: transaction already exists in map by sender, but not in map by hash
-	cache.txListBySender.addTx(createTx([]byte("alice-x"), "alice", 42))
+	cache.txListBySender.addTxReturnEvicted(createTx([]byte("alice-x"), "alice", 42))
 
 	require.False(t, cache.Has([]byte("alice-x")))
 	ok, added = cache.AddTx(createTx([]byte("alice-x"), "alice", 42))

txcache/txListBySenderMap.go

@@ -31,12 +31,22 @@ func newTxListBySenderMap(
 	}
 }
 
-// addTx adds a transaction in the map, in the corresponding list (selected by its sender)
-func (txMap *txListBySenderMap) addTx(tx *WrappedTransaction) (bool, [][]byte) {
+// addTxReturnEvicted adds a transaction in the map, in the corresponding list (selected by its sender).
+// This function returns a boolean indicating whether the transaction was added, and a slice of evicted transaction hashes (upon applying sender-level constraints).
+func (txMap *txListBySenderMap) addTxReturnEvicted(tx *WrappedTransaction) (bool, [][]byte) {
 	sender := string(tx.Tx.GetSndAddr())
 	listForSender := txMap.getOrAddListForSender(sender)
 	tx.computePricePerGasUnit(txMap.txGasHandler)
-	return listForSender.AddTx(tx)
+
+	added, evictedHashes := listForSender.AddTx(tx)
+
+	if listForSender.IsEmpty() {
+		// Generally speaking, a sender cannot become empty after upon applying sender-level constraints.
+		// However:
+		txMap.removeSender(sender)
+	}
+
+	return added, evictedHashes
 }
 
 // getOrAddListForSender gets or lazily creates a list (using double-checked locking pattern)

txcache/txListBySenderMap_test.go

@@ -12,9 +12,9 @@ import (
 func TestSendersMap_AddTx_IncrementsCounter(t *testing.T) {
 	myMap := newSendersMapToTest()
 
-	myMap.addTx(createTx([]byte("a"), "alice", 1))
-	myMap.addTx(createTx([]byte("aa"), "alice", 2))
-	myMap.addTx(createTx([]byte("b"), "bob", 1))
+	myMap.addTxReturnEvicted(createTx([]byte("a"), "alice", 1))
+	myMap.addTxReturnEvicted(createTx([]byte("aa"), "alice", 2))
+	myMap.addTxReturnEvicted(createTx([]byte("b"), "bob", 1))
 
 	// There are 2 senders
 	require.Equal(t, int64(2), myMap.counter.Get())
@@ -27,9 +27,9 @@ func TestSendersMap_RemoveTx_AlsoRemovesSenderWhenNoTransactionLeft(t *testing.T
 	txAlice2 := createTx([]byte("a2"), "alice", 2)
 	txBob := createTx([]byte("b"), "bob", 1)
 
-	myMap.addTx(txAlice1)
-	myMap.addTx(txAlice2)
-	myMap.addTx(txBob)
+	myMap.addTxReturnEvicted(txAlice1)
+	myMap.addTxReturnEvicted(txAlice2)
+	myMap.addTxReturnEvicted(txBob)
 	require.Equal(t, int64(2), myMap.counter.Get())
 	require.Equal(t, uint64(2), myMap.testGetListForSender("alice").countTx())
 	require.Equal(t, uint64(1), myMap.testGetListForSender("bob").countTx())
@@ -51,7 +51,7 @@ func TestSendersMap_RemoveTx_AlsoRemovesSenderWhenNoTransactionLeft(t *testing.T
 func TestSendersMap_RemoveSender(t *testing.T) {
 	myMap := newSendersMapToTest()
 
-	myMap.addTx(createTx([]byte("a"), "alice", 1))
+	myMap.addTxReturnEvicted(createTx([]byte("a"), "alice", 1))
 	require.Equal(t, int64(1), myMap.counter.Get())
 
 	// Bob is unknown
@@ -86,9 +86,9 @@ func TestSendersMap_RemoveSendersBulk_ConcurrentWithAddition(t *testing.T) {
 	wg.Add(100)
 	for i := 0; i < 100; i++ {
 		go func(i int) {
-			myMap.addTx(createTx([]byte("a"), "alice", uint64(i)))
-			myMap.addTx(createTx([]byte("b"), "bob", uint64(i)))
-			myMap.addTx(createTx([]byte("c"), "carol", uint64(i)))
+			myMap.addTxReturnEvicted(createTx([]byte("a"), "alice", uint64(i)))
+			myMap.addTxReturnEvicted(createTx([]byte("b"), "bob", uint64(i)))
+			myMap.addTxReturnEvicted(createTx([]byte("c"), "carol", uint64(i)))
 
 			wg.Done()
 		}(i)
@@ -103,7 +103,7 @@ func TestSendersMap_notifyAccountNonce(t *testing.T) {
 	// Discarded notification, since sender not added yet
 	myMap.notifyAccountNonceReturnEvictedTransactions([]byte("alice"), 42)
 
-	myMap.addTx(createTx([]byte("tx-42"), "alice", 42))
+	myMap.addTxReturnEvicted(createTx([]byte("tx-42"), "alice", 42))
 	alice, _ := myMap.getListForSender("alice")
 	require.Equal(t, uint64(0), alice.accountNonce.Get())
 	require.False(t, alice.accountNonceKnown.IsSet())

txcache/txListForSender.go

@@ -50,7 +50,6 @@ func (listForSender *txListForSender) AddTx(tx *WrappedTransaction) (bool, [][]b
 
 	listForSender.onAddedTransaction(tx)
 
-	// TODO: Check how does the sender get removed if empty afterwards (maybe the answer is: "it never gets empty after applySizeConstraints()").
 	evicted := listForSender.applySizeConstraints()
 	return true, evicted
 }