diff --git a/interactors/errors.go b/interactors/errors.go index 5beef4ef..b5c52912 100644 --- a/interactors/errors.go +++ b/interactors/errors.go @@ -29,5 +29,8 @@ var ErrNilTransaction = errors.New("nil transaction") // ErrTxAlreadySent signals that a transaction was already sent var ErrTxAlreadySent = errors.New("transaction already sent") +// ErrTxWithSameNonceAndGasPriceAlreadySent signals that a transaction with the same nonce & gas price was already sent +var ErrTxWithSameNonceAndGasPriceAlreadySent = errors.New("transaction with the same nonce & gas price was already sent") + // ErrGapNonce signals that a gap nonce between the lowest nonce of the transactions from the cache and the blockchain nonce has been detected var ErrGapNonce = errors.New("gap nonce detected") diff --git a/interactors/nonceHandlerV2/addressNonceHandler.go b/interactors/nonceHandlerV2/addressNonceHandler.go index 2adcfd27..651a4911 100644 --- a/interactors/nonceHandlerV2/addressNonceHandler.go +++ b/interactors/nonceHandlerV2/addressNonceHandler.go @@ -1,7 +1,6 @@ package nonceHandlerV2 import ( - "bytes" "context" "sync" @@ -48,9 +47,9 @@ func NewAddressNonceHandler(proxy interactors.Proxy, address sdkCore.AddressHand // ApplyNonceAndGasPrice will apply the computed nonce to the given FrontendTransaction func (anh *addressNonceHandler) ApplyNonceAndGasPrice(ctx context.Context, tx *transaction.FrontendTransaction) error { - oldTx, alreadyExists := anh.isTxAlreadySent(tx) - if alreadyExists { - err := anh.handleTxAlreadyExists(oldTx, tx) + oldTx := anh.getOlderTxWithSameNonce(tx) + if oldTx != nil { + err := anh.handleTxWithSameNonce(oldTx, tx) if err != nil { return err } @@ -67,7 +66,7 @@ func (anh *addressNonceHandler) ApplyNonceAndGasPrice(ctx context.Context, tx *t return nil } -func (anh *addressNonceHandler) handleTxAlreadyExists(oldTx *transaction.FrontendTransaction, tx *transaction.FrontendTransaction) error { +func (anh *addressNonceHandler) handleTxWithSameNonce(oldTx *transaction.FrontendTransaction, tx *transaction.FrontendTransaction) error { if oldTx.GasPrice < tx.GasPrice { return nil } @@ -76,7 +75,7 @@ func (anh *addressNonceHandler) handleTxAlreadyExists(oldTx *transaction.Fronten return nil } - return interactors.ErrTxAlreadySent + return interactors.ErrTxWithSameNonceAndGasPriceAlreadySent } func (anh *addressNonceHandler) fetchGasPriceIfRequired(ctx context.Context, nonce uint64) { @@ -186,19 +185,16 @@ func (anh *addressNonceHandler) DropTransactions() { anh.mut.Unlock() } -func (anh *addressNonceHandler) isTxAlreadySent(tx *transaction.FrontendTransaction) (*transaction.FrontendTransaction, bool) { +func (anh *addressNonceHandler) getOlderTxWithSameNonce(tx *transaction.FrontendTransaction) *transaction.FrontendTransaction { anh.mut.RLock() defer anh.mut.RUnlock() + for _, oldTx := range anh.transactions { - isTheSameReceiverDataValue := tx.Nonce == oldTx.Nonce && - oldTx.Receiver == tx.Receiver && - bytes.Equal(oldTx.Data, tx.Data) && - oldTx.Value == tx.Value - if isTheSameReceiverDataValue { - return oldTx, true + if oldTx.Nonce == tx.Nonce { + return oldTx } } - return nil, false + return nil } // IsInterfaceNil returns true if there is no value under the interface diff --git a/interactors/nonceHandlerV2/addressNonceHandler_test.go b/interactors/nonceHandlerV2/addressNonceHandler_test.go index 1415cc76..8b90bdfa 100644 --- a/interactors/nonceHandlerV2/addressNonceHandler_test.go +++ b/interactors/nonceHandlerV2/addressNonceHandler_test.go @@ -58,12 +58,15 @@ func TestAddressNonceHandler_ApplyNonceAndGasPrice(t *testing.T) { anh, err := NewAddressNonceHandlerWithPrivateAccess(&testsCommon.ProxyStub{}, testAddress) require.Nil(t, err) + err = anh.ApplyNonceAndGasPrice(context.Background(), &tx) + require.Nil(t, err) + _, err = anh.SendTransaction(context.Background(), &tx) require.Nil(t, err) anh.gasPrice = tx.GasPrice err = anh.ApplyNonceAndGasPrice(context.Background(), &tx) - require.Equal(t, interactors.ErrTxAlreadySent, err) + require.Equal(t, interactors.ErrTxWithSameNonceAndGasPriceAlreadySent, err) }) t.Run("tx already sent; oldTx.GasPrice < txArgs.GasPrice", func(t *testing.T) { t.Parallel() @@ -75,12 +78,18 @@ func TestAddressNonceHandler_ApplyNonceAndGasPrice(t *testing.T) { anh, err := NewAddressNonceHandlerWithPrivateAccess(&testsCommon.ProxyStub{}, testAddress) require.Nil(t, err) + err = anh.ApplyNonceAndGasPrice(context.Background(), &tx) + require.Nil(t, err) + _, err = anh.SendTransaction(context.Background(), &tx) require.Nil(t, err) anh.gasPrice = initialGasPrice err = anh.ApplyNonceAndGasPrice(context.Background(), &tx) require.Nil(t, err) + + _, err = anh.SendTransaction(context.Background(), &tx) + require.Nil(t, err) }) t.Run("oldTx.GasPrice == txArgs.GasPrice && oldTx.GasPrice < anh.gasPrice", func(t *testing.T) { t.Parallel() @@ -89,12 +98,18 @@ func TestAddressNonceHandler_ApplyNonceAndGasPrice(t *testing.T) { anh, err := NewAddressNonceHandlerWithPrivateAccess(&testsCommon.ProxyStub{}, testAddress) require.Nil(t, err) + err = anh.ApplyNonceAndGasPrice(context.Background(), &tx) + require.Nil(t, err) + _, err = anh.SendTransaction(context.Background(), &tx) require.Nil(t, err) anh.gasPrice = tx.GasPrice + 1 err = anh.ApplyNonceAndGasPrice(context.Background(), &tx) require.Nil(t, err) + + _, err = anh.SendTransaction(context.Background(), &tx) + require.Nil(t, err) }) t.Run("same transaction but with different nonce should work", func(t *testing.T) { t.Parallel() @@ -106,11 +121,17 @@ func TestAddressNonceHandler_ApplyNonceAndGasPrice(t *testing.T) { anh, err := NewAddressNonceHandlerWithPrivateAccess(&testsCommon.ProxyStub{}, testAddress) require.Nil(t, err) + err = anh.ApplyNonceAndGasPrice(context.Background(), &tx1) + require.Nil(t, err) + _, err = anh.SendTransaction(context.Background(), &tx1) require.Nil(t, err) err = anh.ApplyNonceAndGasPrice(context.Background(), &tx2) require.Nil(t, err) + + _, err = anh.SendTransaction(context.Background(), &tx2) + require.Nil(t, err) }) } diff --git a/interactors/nonceHandlerV2/nonceTransactionsHandler_test.go b/interactors/nonceHandlerV2/nonceTransactionsHandler_test.go index 5e59cd47..104f607d 100644 --- a/interactors/nonceHandlerV2/nonceTransactionsHandler_test.go +++ b/interactors/nonceHandlerV2/nonceTransactionsHandler_test.go @@ -415,7 +415,7 @@ func TestNonceTransactionsHandlerV2_SendDuplicateTransactions(t *testing.T) { // and computedNonce shall not increase tx.Nonce = 0 err = nth.ApplyNonceAndGasPrice(context.Background(), testAddress, tx) - require.Equal(t, err, interactors.ErrTxAlreadySent) + require.Equal(t, err, interactors.ErrTxWithSameNonceAndGasPriceAlreadySent) require.Equal(t, tx.Nonce, uint64(0)) require.Equal(t, accWithPrivateAccess.computedNonce+1, currentNonce) } diff --git a/interactors/nonceHandlerV2/singleTransactionAddressNonceHandler.go b/interactors/nonceHandlerV2/singleTransactionAddressNonceHandler.go deleted file mode 100644 index d771349c..00000000 --- a/interactors/nonceHandlerV2/singleTransactionAddressNonceHandler.go +++ /dev/null @@ -1,125 +0,0 @@ -package nonceHandlerV2 - -import ( - "context" - "sync" - - "github.com/multiversx/mx-chain-core-go/core" - "github.com/multiversx/mx-chain-core-go/core/check" - "github.com/multiversx/mx-chain-core-go/data/transaction" - sdkCore "github.com/multiversx/mx-sdk-go/core" - "github.com/multiversx/mx-sdk-go/interactors" -) - -type singleTransactionAddressNonceHandler struct { - mut sync.RWMutex - address sdkCore.AddressHandler - transaction *transaction.FrontendTransaction - gasPrice uint64 - nonceUntilGasIncreased uint64 - proxy interactors.Proxy -} - -// NewSingleTransactionAddressNonceHandler returns a new instance of a singleTransactionAddressNonceHandler -func NewSingleTransactionAddressNonceHandler(proxy interactors.Proxy, address sdkCore.AddressHandler) (*singleTransactionAddressNonceHandler, error) { - if check.IfNil(proxy) { - return nil, interactors.ErrNilProxy - } - if check.IfNil(address) { - return nil, interactors.ErrNilAddress - } - return &singleTransactionAddressNonceHandler{ - address: address, - proxy: proxy, - }, nil -} - -// ApplyNonceAndGasPrice will apply the computed nonce to the given FrontendTransaction -func (anh *singleTransactionAddressNonceHandler) ApplyNonceAndGasPrice(ctx context.Context, tx *transaction.FrontendTransaction) error { - nonce, err := anh.getNonce(ctx) - if err != nil { - return err - } - tx.Nonce = nonce - - anh.fetchGasPriceIfRequired(ctx, nonce) - tx.GasPrice = core.MaxUint64(anh.gasPrice, tx.GasPrice) - return nil -} - -func (anh *singleTransactionAddressNonceHandler) fetchGasPriceIfRequired(ctx context.Context, nonce uint64) { - if nonce == anh.nonceUntilGasIncreased+1 || anh.gasPrice == 0 { - networkConfig, err := anh.proxy.GetNetworkConfig(ctx) - - anh.mut.Lock() - defer anh.mut.Unlock() - if err != nil { - log.Error("%w: while fetching network config", err) - anh.gasPrice = 0 - return - } - anh.gasPrice = networkConfig.MinGasPrice - } -} - -func (anh *singleTransactionAddressNonceHandler) getNonce(ctx context.Context) (uint64, error) { - account, err := anh.proxy.GetAccount(ctx, anh.address) - if err != nil { - return 0, err - } - - return account.Nonce, nil -} - -// ReSendTransactionsIfRequired will resend the cached transaction if it still has a nonce greater than the one fetched from the blockchain -func (anh *singleTransactionAddressNonceHandler) ReSendTransactionsIfRequired(ctx context.Context) error { - if anh.transaction == nil { - return nil - } - nonce, err := anh.getNonce(ctx) - if err != nil { - return err - } - - if anh.transaction.Nonce != nonce { - anh.transaction = nil - return nil - } - - hash, err := anh.proxy.SendTransaction(ctx, anh.transaction) - if err != nil { - return err - } - - addressAsBech32String, err := anh.address.AddressAsBech32String() - if err != nil { - return err - } - - log.Debug("resent transaction", "address", addressAsBech32String, "hash", hash) - - return nil -} - -// SendTransaction will save and propagate a transaction to the network -func (anh *singleTransactionAddressNonceHandler) SendTransaction(ctx context.Context, tx *transaction.FrontendTransaction) (string, error) { - anh.mut.Lock() - anh.transaction = tx - anh.mut.Unlock() - - return anh.proxy.SendTransaction(ctx, tx) -} - -// DropTransactions will delete the cached transaction and will try to replace the current transaction from the pool using more gas price -func (anh *singleTransactionAddressNonceHandler) DropTransactions() { - anh.mut.Lock() - defer anh.mut.Unlock() - anh.gasPrice++ - anh.nonceUntilGasIncreased = anh.transaction.Nonce - anh.transaction = nil -} - -// IsInterfaceNil returns true if there is no value under the interface -func (anh *singleTransactionAddressNonceHandler) IsInterfaceNil() bool { - return anh == nil -} diff --git a/interactors/nonceHandlerV2/singleTransactionAddressNonceHandler_test.go b/interactors/nonceHandlerV2/singleTransactionAddressNonceHandler_test.go deleted file mode 100644 index 4e276ccd..00000000 --- a/interactors/nonceHandlerV2/singleTransactionAddressNonceHandler_test.go +++ /dev/null @@ -1,244 +0,0 @@ -package nonceHandlerV2 - -import ( - "context" - "crypto/rand" - "testing" - - "github.com/multiversx/mx-chain-core-go/data/transaction" - "github.com/multiversx/mx-sdk-go/core" - "github.com/multiversx/mx-sdk-go/data" - "github.com/multiversx/mx-sdk-go/interactors" - "github.com/multiversx/mx-sdk-go/testsCommon" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestSingleTransactionAddressNonceHandler_NewSingleTransactionAddressNonceHandler(t *testing.T) { - t.Parallel() - - t.Run("nil proxy", func(t *testing.T) { - t.Parallel() - - anh, err := NewSingleTransactionAddressNonceHandler(nil, nil) - assert.Nil(t, anh) - assert.Equal(t, interactors.ErrNilProxy, err) - }) - t.Run("nil addressHandler", func(t *testing.T) { - t.Parallel() - - anh, err := NewSingleTransactionAddressNonceHandler(&testsCommon.ProxyStub{}, nil) - assert.Nil(t, anh) - assert.Equal(t, interactors.ErrNilAddress, err) - }) - t.Run("should work", func(t *testing.T) { - t.Parallel() - - pubkey := make([]byte, 32) - _, _ = rand.Read(pubkey) - addressHandler := data.NewAddressFromBytes(pubkey) - - _, err := NewSingleTransactionAddressNonceHandler(&testsCommon.ProxyStub{}, addressHandler) - assert.Nil(t, err) - }) -} - -func TestSingleTransactionAddressNonceHandler_ApplyNonceAndGasPrice(t *testing.T) { - t.Parallel() - - t.Run("proxy returns error should error", func(t *testing.T) { - t.Parallel() - - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return nil, expectedErr - }, - } - - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - - tx := createDefaultTx() - - err := anh.ApplyNonceAndGasPrice(context.Background(), &tx) - require.Equal(t, expectedErr, err) - }) - t.Run("should work", func(t *testing.T) { - t.Parallel() - - blockchainNonce := uint64(100) - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return &data.Account{Nonce: blockchainNonce}, nil - }, - } - - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - - tx := createDefaultTx() - - err := anh.ApplyNonceAndGasPrice(context.Background(), &tx) - require.Nil(t, err) - require.Equal(t, blockchainNonce, tx.Nonce) - }) -} - -func TestSingleTransactionAddressNonceHandler_fetchGasPriceIfRequired(t *testing.T) { - t.Parallel() - - //proxy returns error should set invalid gasPrice(0) - proxy := &testsCommon.ProxyStub{ - GetNetworkConfigCalled: func() (*data.NetworkConfig, error) { - return nil, expectedErr - }, - } - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - anh.gasPrice = 100000 - anh.nonceUntilGasIncreased = 100 - - anh.fetchGasPriceIfRequired(context.Background(), 101) - require.Equal(t, uint64(0), anh.gasPrice) -} - -func TestSingleTransactionAddressNonceHandler_DropTransactions(t *testing.T) { - t.Parallel() - - tx := createDefaultTx() - - blockchainNonce := uint64(100) - minGasPrice := uint64(10) - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return &data.Account{Nonce: blockchainNonce}, nil - }, - GetNetworkConfigCalled: func() (*data.NetworkConfig, error) { - return &data.NetworkConfig{MinGasPrice: minGasPrice}, nil - }, - } - - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - - err := anh.ApplyNonceAndGasPrice(context.Background(), &tx) - require.Nil(t, err) - - _, err = anh.SendTransaction(context.Background(), &tx) - require.Nil(t, err) - - require.Equal(t, uint64(0), anh.nonceUntilGasIncreased) - require.Equal(t, minGasPrice, anh.gasPrice) - require.NotNil(t, anh.transaction) - - anh.DropTransactions() - - require.Equal(t, blockchainNonce, anh.nonceUntilGasIncreased) - require.Equal(t, minGasPrice+1, anh.gasPrice) - require.Nil(t, anh.transaction) -} - -func TestSingleTransactionAddressNonceHandler_SendTransaction(t *testing.T) { - t.Parallel() - - // proxy returns error should error - proxy := &testsCommon.ProxyStub{ - SendTransactionCalled: func(tx *transaction.FrontendTransaction) (string, error) { - return "", expectedErr - }, - } - - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - - tx := createDefaultTx() - - _, err := anh.SendTransaction(context.Background(), &tx) - require.Equal(t, expectedErr, err) -} - -func TestSingleTransactionAddressNonceHandler_ReSendTransactionsIfRequired(t *testing.T) { - t.Parallel() - - t.Run("no transaction to resend shall exit early with no error", func(t *testing.T) { - t.Parallel() - - anh, _ := NewSingleTransactionAddressNonceHandler(&testsCommon.ProxyStub{}, testAddress) - err := anh.ReSendTransactionsIfRequired(context.Background()) - require.Nil(t, err) - }) - t.Run("proxy returns error shall error", func(t *testing.T) { - t.Parallel() - - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return nil, expectedErr - }, - } - - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - tx := createDefaultTx() - - _, err := anh.SendTransaction(context.Background(), &tx) - require.Nil(t, err) - - err = anh.ReSendTransactionsIfRequired(context.Background()) - require.Equal(t, expectedErr, err) - }) - t.Run("proxy returns error shall error", func(t *testing.T) { - t.Parallel() - - blockchainNonce := uint64(100) - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return &data.Account{Nonce: blockchainNonce}, nil - }, - SendTransactionCalled: func(txs *transaction.FrontendTransaction) (string, error) { - return "", expectedErr - }, - } - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - tx := createDefaultTx() - tx.Nonce = blockchainNonce - anh.transaction = &tx - - err := anh.ReSendTransactionsIfRequired(context.Background()) - require.Equal(t, expectedErr, err) - }) - t.Run("anh.transaction.Nonce != account.Nonce", func(t *testing.T) { - t.Parallel() - - blockchainNonce := uint64(100) - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return &data.Account{Nonce: blockchainNonce + 1}, nil - }, - SendTransactionCalled: func(txs *transaction.FrontendTransaction) (string, error) { - return "", expectedErr - }, - } - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - tx := createDefaultTx() - tx.Nonce = blockchainNonce - anh.transaction = &tx - - err := anh.ReSendTransactionsIfRequired(context.Background()) - require.Nil(t, err) - require.Nil(t, anh.transaction) - }) - t.Run("should work", func(t *testing.T) { - t.Parallel() - - blockchainNonce := uint64(100) - proxy := &testsCommon.ProxyStub{ - GetAccountCalled: func(address core.AddressHandler) (*data.Account, error) { - return &data.Account{Nonce: blockchainNonce}, nil - }, - SendTransactionCalled: func(txs *transaction.FrontendTransaction) (string, error) { - return "hash", nil - }, - } - anh, _ := NewSingleTransactionAddressNonceHandler(proxy, testAddress) - tx := createDefaultTx() - tx.Nonce = blockchainNonce - anh.transaction = &tx - - err := anh.ReSendTransactionsIfRequired(context.Background()) - require.Nil(t, err) - }) -}