Fix after review (part 1).

Author: andreibancioiu

testscommon/txcachemocks/accountStateProviderMock.go

@@ -2,12 +2,15 @@ package txcachemocks
 
 import (
 	"math/big"
+	"sync"
 
 	"github.com/multiversx/mx-chain-storage-go/types"
 )
 
 // AccountStateProviderMock -
 type AccountStateProviderMock struct {
+	mutex sync.Mutex
+
 	AccountStateByAddress map[string]*types.AccountState
 	GetAccountStateCalled func(address []byte) (*types.AccountState, error)
 }
@@ -21,6 +24,9 @@ func NewAccountStateProviderMock() *AccountStateProviderMock {
 
 // SetNonce -
 func (mock *AccountStateProviderMock) SetNonce(address []byte, nonce uint64) {
+	mock.mutex.Lock()
+	defer mock.mutex.Unlock()
+
 	key := string(address)
 
 	if mock.AccountStateByAddress[key] == nil {
@@ -32,28 +38,23 @@ func (mock *AccountStateProviderMock) SetNonce(address []byte, nonce uint64) {
 
 // SetBalance -
 func (mock *AccountStateProviderMock) SetBalance(address []byte, balance *big.Int) {
-	key := string(address)
-
-	if mock.AccountStateByAddress[key] == nil {
-		mock.AccountStateByAddress[key] = newDefaultAccountState()
-	}
-
-	mock.AccountStateByAddress[key].Balance = balance
-}
+	mock.mutex.Lock()
+	defer mock.mutex.Unlock()
 
-// SetGuardian -
-func (mock *AccountStateProviderMock) SetGuardian(address []byte, guardian []byte) {
 	key := string(address)
 
 	if mock.AccountStateByAddress[key] == nil {
 		mock.AccountStateByAddress[key] = newDefaultAccountState()
 	}
 
-	mock.AccountStateByAddress[key].Guardian = guardian
+	mock.AccountStateByAddress[key].Balance = balance
 }
 
 // GetAccountState -
 func (mock *AccountStateProviderMock) GetAccountState(address []byte) (*types.AccountState, error) {
+	mock.mutex.Lock()
+	defer mock.mutex.Unlock()
+
 	if mock.GetAccountStateCalled != nil {
 		return mock.GetAccountStateCalled(address)
 	}
@@ -73,8 +74,7 @@ func (mock *AccountStateProviderMock) IsInterfaceNil() bool {
 
 func newDefaultAccountState() *types.AccountState {
 	return &types.AccountState{
-		Nonce:    0,
-		Balance:  big.NewInt(1000000000000000000),
-		Guardian: nil,
+		Nonce:   0,
+		Balance: big.NewInt(1000000000000000000),
 	}
 }

txcache/selection_test.go

@@ -221,26 +221,6 @@ func TestTxCache_SelectTransactions_HandlesNotExecutableTransactions(t *testing.
 		require.Len(t, sorted, expectedNumSelected)
 		require.Equal(t, 200000, int(accumulatedGas))
 	})
-
-	t.Run("with guardians", func(t *testing.T) {
-		cache := newUnconstrainedCacheToTest()
-		accountStateProvider := txcachemocks.NewAccountStateProviderMock()
-		accountStateProvider.SetNonce([]byte("alice"), 1)
-		accountStateProvider.SetNonce([]byte("bob"), 42)
-		accountStateProvider.SetGuardian([]byte("bob"), []byte("heidi"))
-
-		cache.AddTx(createTx([]byte("hash-alice-1"), "alice", 1))
-		cache.AddTx(createTx([]byte("hash-bob-42a"), "bob", 42))
-		cache.AddTx(createTx([]byte("hash-bob-42b"), "bob", 42).withGuardian([]byte("heidi")).withGasLimit(100000))
-		cache.AddTx(createTx([]byte("hash-bob-43"), "bob", 43).withGuardian([]byte("grace")).withGasLimit(100000))
-
-		sorted, accumulatedGas := cache.SelectTransactions(accountStateProvider, math.MaxUint64, math.MaxInt, selectionLoopMaximumDuration)
-		require.Len(t, sorted, 2)
-		require.Equal(t, 150000, int(accumulatedGas))
-
-		require.Equal(t, "hash-alice-1", string(sorted[0].TxHash))
-		require.Equal(t, "hash-bob-42b", string(sorted[1].TxHash))
-	})
 }
 
 func TestTxCache_SelectTransactions_WhenTransactionsAddedInReversedNonceOrder(t *testing.T) {

txcache/transactionsHeapItem.go

@@ -1,7 +1,6 @@
 package txcache
 
 import (
-	"bytes"
 	"math/big"
 
 	"github.com/multiversx/mx-chain-storage-go/types"
@@ -159,24 +158,8 @@ func (item *transactionsHeapItem) detectLowerNonce() bool {
 }
 
 func (item *transactionsHeapItem) detectBadlyGuarded() bool {
-	if item.senderState == nil {
-		return false
-	}
-
-	transactionGuardian := item.currentTransaction.Guardian
-	accountGuardian := item.senderState.Guardian
-
-	isBadlyGuarded := !bytes.Equal(transactionGuardian, accountGuardian)
-	if isBadlyGuarded {
-		logSelect.Trace("transactionsHeapItem.detectBadlyGuarded",
-			"tx", item.currentTransaction.TxHash,
-			"sender", item.sender,
-			"transactionGuardian", transactionGuardian,
-			"accountGuardian", accountGuardian,
-		)
-	}
-
-	return isBadlyGuarded
+	// See MX-16179.
+	return false
 }
 
 func (item *transactionsHeapItem) detectNonceDuplicate() bool {

txcache/transactionsHeapItem_test.go

@@ -199,78 +199,6 @@ func TestTransactionsHeapItem_detectLowerNonce(t *testing.T) {
 	})
 }
 
-func TestTransactionsHeapItem_detectBadlyGuarded(t *testing.T) {
-	txGasHandler := txcachemocks.NewTxGasHandlerMock()
-
-	a := createTx([]byte("tx-1"), "alice", 42)
-	b := createTx([]byte("tx-7"), "bob", 43).withGuardian([]byte("heidi"))
-
-	a.precomputeFields(txGasHandler)
-	b.precomputeFields(txGasHandler)
-
-	t.Run("unknown", func(t *testing.T) {
-		item, err := newTransactionsHeapItem(bunchOfTransactions{a})
-		require.NoError(t, err)
-
-		require.False(t, item.detectBadlyGuarded())
-	})
-
-	t.Run("transaction has no guardian, account has no guardian", func(t *testing.T) {
-		item, err := newTransactionsHeapItem(bunchOfTransactions{a})
-		require.NoError(t, err)
-
-		item.senderState = &types.AccountState{
-			Guardian: nil,
-		}
-
-		require.False(t, item.detectBadlyGuarded())
-	})
-
-	t.Run("transaction has guardian, account has guardian, they match", func(t *testing.T) {
-		item, err := newTransactionsHeapItem(bunchOfTransactions{b})
-		require.NoError(t, err)
-
-		item.senderState = &types.AccountState{
-			Guardian: []byte("heidi"),
-		}
-
-		require.False(t, item.detectBadlyGuarded())
-	})
-
-	t.Run("transaction has guardian, account has guardian, they don't match", func(t *testing.T) {
-		item, err := newTransactionsHeapItem(bunchOfTransactions{b})
-		require.NoError(t, err)
-
-		item.senderState = &types.AccountState{
-			Guardian: []byte("grace"),
-		}
-
-		require.True(t, item.detectBadlyGuarded())
-	})
-
-	t.Run("transaction has guardian, account does not", func(t *testing.T) {
-		item, err := newTransactionsHeapItem(bunchOfTransactions{b})
-		require.NoError(t, err)
-
-		item.senderState = &types.AccountState{
-			Guardian: nil,
-		}
-
-		require.True(t, item.detectBadlyGuarded())
-	})
-
-	t.Run("transaction has no guardian, account has guardian", func(t *testing.T) {
-		item, err := newTransactionsHeapItem(bunchOfTransactions{a})
-		require.NoError(t, err)
-
-		item.senderState = &types.AccountState{
-			Guardian: []byte("heidi"),
-		}
-
-		require.True(t, item.detectBadlyGuarded())
-	})
-}
-
 func TestTransactionsHeapItem_detectNonceDuplicate(t *testing.T) {
 	a := createTx([]byte("tx-1"), "alice", 42)
 	b := createTx([]byte("tx-2"), "alice", 43)

txcache/wrappedTransaction.go

@@ -23,7 +23,6 @@ type WrappedTransaction struct {
 	// Additional note: "WrappedTransaction" objects are created by the Node, in dataRetriever/txpool/shardedTxPool.go.
 	Fee          *big.Int
 	PricePerUnit uint64
-	Guardian     []byte
 }
 
 // precomputeFields computes (and caches) the (average) price per gas unit.
@@ -34,11 +33,6 @@ func (wrappedTx *WrappedTransaction) precomputeFields(txGasHandler TxGasHandler)
 	if gasLimit != 0 {
 		wrappedTx.PricePerUnit = wrappedTx.Fee.Uint64() / gasLimit
 	}
-
-	txAsGuardedTransaction, ok := wrappedTx.Tx.(data.GuardedTransactionHandler)
-	if ok {
-		wrappedTx.Guardian = txAsGuardedTransaction.GetGuardianAddr()
-	}
 }
 
 // Equality is out of scope (not possible in our case).

txcache/wrappedTransaction_test.go

@@ -16,27 +16,24 @@ func TestWrappedTransaction_precomputeFields(t *testing.T) {
 
 		require.Equal(t, "51500000000000", tx.Fee.String())
 		require.Equal(t, oneBillion, int(tx.PricePerUnit))
-		require.Empty(t, tx.Guardian)
 	})
 
-	t.Run("move balance gas limit and execution gas limit (1)", func(t *testing.T) {
+	t.Run("move balance gas limit and execution gas limit (a)", func(t *testing.T) {
 		tx := createTx([]byte("b"), "b", 1).withDataLength(1).withGasLimit(51501).withGasPrice(oneBillion)
 		tx.precomputeFields(txGasHandler)
 
 		require.Equal(t, "51500010000000", tx.Fee.String())
 		require.Equal(t, 999_980_777, int(tx.PricePerUnit))
-		require.Empty(t, tx.Guardian)
 	})
 
-	t.Run("move balance gas limit and execution gas limit (2)", func(t *testing.T) {
+	t.Run("move balance gas limit and execution gas limit (b)", func(t *testing.T) {
 		tx := createTx([]byte("c"), "c", 1).withDataLength(1).withGasLimit(oneMilion).withGasPrice(oneBillion)
 		tx.precomputeFields(txGasHandler)
 
 		actualFee := 51500*oneBillion + (oneMilion-51500)*oneBillion/100
 		require.Equal(t, "60985000000000", tx.Fee.String())
 		require.Equal(t, 60_985_000_000_000, actualFee)
 		require.Equal(t, actualFee/oneMilion, int(tx.PricePerUnit))
-		require.Empty(t, tx.Guardian)
 	})
 
 	t.Run("with guardian", func(t *testing.T) {
@@ -45,7 +42,6 @@ func TestWrappedTransaction_precomputeFields(t *testing.T) {
 
 		require.Equal(t, "50000000000000", tx.Fee.String())
 		require.Equal(t, oneBillion, int(tx.PricePerUnit))
-		require.Equal(t, []byte("heidi"), tx.Guardian)
 	})
 }
 

types/accountState.go

@@ -4,7 +4,6 @@ import "math/big"
 
 // AccountState represents the state of an account, as seen by the mempool
 type AccountState struct {
-	Nonce    uint64
-	Balance  *big.Int
-	Guardian []byte
+	Nonce   uint64
+	Balance *big.Int
 }