fix: unlock local nonce when an error occurs and increment only when the tx is sent for real

services/wallet/bridge/bridge.go

@@ -96,5 +96,5 @@ type Bridge interface {
 	CalculateAmountOut(from, to *params.Network, amountIn *big.Int, symbol string) (*big.Int, error)
 	Send(sendArgs *TransactionBridge, verifiedAccount *account.SelectedExtKey) (types.Hash, error)
 	GetContractAddress(network *params.Network, token *token.Token) *common.Address
-	BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, error)
+	BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, transactions.UnlockNonceFunc, error)
 }

services/wallet/bridge/cbridge.go

@@ -312,8 +312,8 @@ func (s *CBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerF
 	if fromNetwork == nil {
 		return nil, errors.New("network not found")
 	}
-	tk := s.tokenManager.FindToken(fromNetwork, sendArgs.CbridgeTx.Symbol)
+	token := s.tokenManager.FindToken(fromNetwork, sendArgs.CbridgeTx.Symbol)
-	if tk == nil {
+	if token == nil {
 		return nil, errors.New("token not found")
 	}
 	addrs := s.GetContractAddress(fromNetwork, nil)
@@ -331,7 +331,7 @@ func (s *CBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerF
 	}
 
 	txOpts := sendArgs.CbridgeTx.ToTransactOpts(signerFn)
-	if tk.IsNative() {
+	if token.IsNative() {
 		return contract.SendNative(
 			txOpts,
 			sendArgs.CbridgeTx.Recipient,
@@ -345,7 +345,7 @@ func (s *CBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerF
 	return contract.Send(
 		txOpts,
 		sendArgs.CbridgeTx.Recipient,
-		tk.Address,
+		token.Address,
 		(*big.Int)(sendArgs.CbridgeTx.Amount),
 		sendArgs.CbridgeTx.ChainID,
 		uint64(time.Now().UnixMilli()),
@@ -362,8 +362,9 @@ func (s *CBridge) Send(sendArgs *TransactionBridge, verifiedAccount *account.Sel
 	return types.Hash(tx.Hash()), nil
 }
 
-func (s *CBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, error) {
+func (s *CBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, transactions.UnlockNonceFunc, error) {
-	return s.sendOrBuild(sendArgs, nil)
+	tx, err := s.sendOrBuild(sendArgs, nil)
+	return tx, nil, err
 }
 
 func (s *CBridge) CalculateAmountOut(from, to *params.Network, amountIn *big.Int, symbol string) (*big.Int, error) {

services/wallet/bridge/erc721_transfer.go

@@ -95,40 +95,45 @@ func (s *ERC721TransferBridge) EstimateGas(fromNetwork *params.Network, toNetwor
 	return uint64(increasedEstimation), nil
 }
 
-func (s *ERC721TransferBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerFn) (tx *ethTypes.Transaction, err error) {
+func (s *ERC721TransferBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerFn) (tx *ethTypes.Transaction, unlock transactions.UnlockNonceFunc, err error) {
 	ethClient, err := s.rpcClient.EthClient(sendArgs.ChainID)
 	if err != nil {
-		return tx, err
+		return tx, nil, err
 	}
 
 	contract, err := collectibles.NewCollectibles(common.Address(*sendArgs.ERC721TransferTx.To), ethClient)
 	if err != nil {
-		return tx, err
+		return tx, nil, err
 	}
 
 	nonce, unlock, err := s.transactor.NextNonce(s.rpcClient, sendArgs.ChainID, sendArgs.ERC721TransferTx.From)
 	if err != nil {
-		return tx, err
+		return tx, nil, err
 	}
-	defer func() {
+
-		unlock(err == nil, nonce)
-	}()
 	argNonce := hexutil.Uint64(nonce)
 	sendArgs.ERC721TransferTx.Nonce = &argNonce
 	txOpts := sendArgs.ERC721TransferTx.ToTransactOpts(signerFn)
-	return contract.SafeTransferFrom(txOpts, common.Address(sendArgs.ERC721TransferTx.From), sendArgs.ERC721TransferTx.Recipient,
+	tx, err = contract.SafeTransferFrom(txOpts, common.Address(sendArgs.ERC721TransferTx.From),
+		sendArgs.ERC721TransferTx.Recipient,
 		sendArgs.ERC721TransferTx.TokenID.ToInt())
+	return tx, unlock, err
 }
 
 func (s *ERC721TransferBridge) Send(sendArgs *TransactionBridge, verifiedAccount *account.SelectedExtKey) (hash types.Hash, err error) {
-	tx, err := s.sendOrBuild(sendArgs, getSigner(sendArgs.ChainID, sendArgs.ERC721TransferTx.From, verifiedAccount))
+	tx, unlock, err := s.sendOrBuild(sendArgs, getSigner(sendArgs.ChainID, sendArgs.ERC721TransferTx.From, verifiedAccount))
+	defer func() {
+		if unlock != nil {
+			unlock(err == nil, tx.Nonce())
+		}
+	}()
 	if err != nil {
 		return hash, err
 	}
 	return types.Hash(tx.Hash()), nil
 }
 
-func (s *ERC721TransferBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, error) {
+func (s *ERC721TransferBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, transactions.UnlockNonceFunc, error) {
 	return s.sendOrBuild(sendArgs, nil)
 }
 

services/wallet/bridge/hop.go

@@ -226,40 +226,43 @@ func (h *HopBridge) GetContractAddress(network *params.Network, token *token.Tok
 	return &address
 }
 
-func (h *HopBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerFn) (tx *ethTypes.Transaction, err error) {
+func (h *HopBridge) sendOrBuild(sendArgs *TransactionBridge, signerFn bind.SignerFn) (tx *ethTypes.Transaction, unlock transactions.UnlockNonceFunc, err error) {
 	fromNetwork := h.contractMaker.RPCClient.NetworkManager.Find(sendArgs.ChainID)
 	if fromNetwork == nil {
-		return tx, err
+		return tx, nil, err
 	}
 
 	nonce, unlock, err := h.transactor.NextNonce(h.contractMaker.RPCClient, sendArgs.ChainID, sendArgs.HopTx.From)
 	if err != nil {
-		return tx, err
+		return tx, nil, err
 	}
-	defer func() {
+
-		unlock(err == nil, nonce)
-	}()
 	argNonce := hexutil.Uint64(nonce)
 	sendArgs.HopTx.Nonce = &argNonce
 
 	token := h.tokenManager.FindToken(fromNetwork, sendArgs.HopTx.Symbol)
 	if fromNetwork.Layer == 1 {
 		tx, err = h.sendToL2(sendArgs.ChainID, sendArgs.HopTx, signerFn, token)
-		return tx, err
+		return tx, unlock, err
 	}
 	tx, err = h.swapAndSend(sendArgs.ChainID, sendArgs.HopTx, signerFn, token)
-	return tx, err
+	return tx, unlock, err
 }
 
 func (h *HopBridge) Send(sendArgs *TransactionBridge, verifiedAccount *account.SelectedExtKey) (hash types.Hash, err error) {
-	tx, err := h.sendOrBuild(sendArgs, getSigner(sendArgs.ChainID, sendArgs.HopTx.From, verifiedAccount))
+	tx, unlock, err := h.sendOrBuild(sendArgs, getSigner(sendArgs.ChainID, sendArgs.HopTx.From, verifiedAccount))
+	defer func() {
+		if unlock != nil {
+			unlock(err == nil, tx.Nonce())
+		}
+	}()
 	if err != nil {
 		return types.Hash{}, err
 	}
 	return types.Hash(tx.Hash()), nil
 }
 
-func (h *HopBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, error) {
+func (h *HopBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, transactions.UnlockNonceFunc, error) {
 	return h.sendOrBuild(sendArgs, nil)
 }
 

services/wallet/bridge/transfer.go

@@ -45,7 +45,7 @@ func (s *TransferBridge) Send(sendArgs *TransactionBridge, verifiedAccount *acco
 	return s.transactor.SendTransactionWithChainID(sendArgs.ChainID, *sendArgs.TransferTx, verifiedAccount)
 }
 
-func (s *TransferBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, error) {
+func (s *TransferBridge) BuildTransaction(sendArgs *TransactionBridge) (*ethTypes.Transaction, transactions.UnlockNonceFunc, error) {
 	return s.transactor.ValidateAndBuildTransaction(sendArgs.ChainID, *sendArgs.TransferTx)
 }
 

services/wallet/transfer/transaction.go

@@ -48,6 +48,7 @@ type TransactionDescription struct {
 	chainID   uint64
 	builtTx   *ethTypes.Transaction
 	signature []byte
+	unlock    transactions.UnlockNonceFunc
 }
 
 type TransactionManager struct {
@@ -385,7 +386,7 @@ func (tm *TransactionManager) ProceedWithTransactionsSignatures(ctx context.Cont
 		rBytes, _ := hex.DecodeString(sigDetails.R)
 		sBytes, _ := hex.DecodeString(sigDetails.S)
 		vByte := byte(0)
-		if sigDetails.V == "1" {
+		if sigDetails.V == "01" {
 			vByte = 1
 		}
 
@@ -399,6 +400,9 @@ func (tm *TransactionManager) ProceedWithTransactionsSignatures(ctx context.Cont
 	hashes := make(map[uint64][]types.Hash)
 	for _, desc := range tm.transactionsForKeycardSingning {
 		hash, err := tm.transactor.SendBuiltTransactionWithSignature(desc.chainID, desc.builtTx, desc.signature)
+		defer func() {
+			desc.unlock(err == nil, desc.builtTx.Nonce())
+		}()
 		if err != nil {
 			return nil, err
 		}
@@ -477,8 +481,11 @@ func (tm *TransactionManager) buildTransactions(bridges map[string]bridge.Bridge
 	tm.transactionsForKeycardSingning = make(map[common.Hash]*TransactionDescription)
 	var hashes []string
 	for _, bridgeTx := range tm.transactionsBridgeData {
-		builtTx, err := bridges[bridgeTx.BridgeName].BuildTransaction(bridgeTx)
+		builtTx, unlock, err := bridges[bridgeTx.BridgeName].BuildTransaction(bridgeTx)
 		if err != nil {
+			if unlock != nil {
+				unlock(false, 0) // unlock nonce in case of an error, otherwise keep it locked, until the transaction is sent
+			}
 			return hashes, err
 		}
 
@@ -488,6 +495,7 @@ func (tm *TransactionManager) buildTransactions(bridges map[string]bridge.Bridge
 		tm.transactionsForKeycardSingning[txHash] = &TransactionDescription{
 			chainID: bridgeTx.ChainID,
 			builtTx: builtTx,
+			unlock:  unlock,
 		}
 
 		hashes = append(hashes, txHash.String())

transactions/nonce.go

@@ -8,6 +8,8 @@ import (
 	"github.com/status-im/status-go/eth-node/types"
 )
 
+type UnlockNonceFunc func(inc bool, n uint64)
+
 type Nonce struct {
 	addrLock   *AddrLocker
 	localNonce map[uint64]*sync.Map
@@ -20,7 +22,7 @@ func NewNonce() *Nonce {
 	}
 }
 
-func (n *Nonce) Next(rpcWrapper *rpcWrapper, from types.Address) (uint64, func(inc bool, nonce uint64), error) {
+func (n *Nonce) Next(rpcWrapper *rpcWrapper, from types.Address) (uint64, UnlockNonceFunc, error) {
 	n.addrLock.LockAddr(from)
 	current, err := n.GetCurrent(rpcWrapper, from)
 	unlock := func(inc bool, nonce uint64) {

transactions/transactor.go

@@ -77,10 +77,11 @@ func (t *Transactor) SetRPC(rpcClient *rpc.Client, timeout time.Duration) {
 	t.rpcCallTimeout = timeout
 }
 
-func (t *Transactor) NextNonce(rpcClient *rpc.Client, chainID uint64, from types.Address) (uint64, func(inc bool, n uint64), error) {
+func (t *Transactor) NextNonce(rpcClient *rpc.Client, chainID uint64, from types.Address) (uint64, UnlockNonceFunc, error) {
 	wrapper := newRPCWrapper(rpcClient, chainID)
 	return t.nonce.Next(wrapper, from)
 }
+
 func (t *Transactor) EstimateGas(network *params.Network, from common.Address, to common.Address, value *big.Int, input []byte) (uint64, error) {
 	rpcWrapper := newRPCWrapper(t.rpcWrapper.RPCClient, network.ChainID)
 
@@ -109,9 +110,9 @@ func (t *Transactor) SendTransactionWithChainID(chainID uint64, sendArgs SendTxA
 	return
 }
 
-func (t *Transactor) ValidateAndBuildTransaction(chainID uint64, sendArgs SendTxArgs) (tx *gethtypes.Transaction, err error) {
+func (t *Transactor) ValidateAndBuildTransaction(chainID uint64, sendArgs SendTxArgs) (tx *gethtypes.Transaction, unlock UnlockNonceFunc, err error) {
 	wrapper := newRPCWrapper(t.rpcWrapper.RPCClient, chainID)
-	tx, err = t.validateAndBuildTransaction(wrapper, sendArgs)
+	tx, unlock, err = t.validateAndBuildTransaction(wrapper, sendArgs)
 	return
 }
 
@@ -267,21 +268,18 @@ func (t *Transactor) validateAccount(args SendTxArgs, selectedAccount *account.S
 	return nil
 }
 
-func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args SendTxArgs) (tx *gethtypes.Transaction, err error) {
+func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args SendTxArgs) (tx *gethtypes.Transaction, unlock UnlockNonceFunc, err error) {
 	if !args.Valid() {
-		return tx, ErrInvalidSendTxArgs
+		return tx, nil, ErrInvalidSendTxArgs
 	}
 
 	nonce, unlock, err := t.nonce.Next(rpcWrapper, args.From)
 	if err != nil {
-		return tx, err
+		return tx, nil, err
 	}
 	if args.Nonce != nil {
 		nonce = uint64(*args.Nonce)
 	}
-	defer func() {
-		unlock(err == nil, nonce)
-	}()
 	ctx, cancel := context.WithTimeout(context.Background(), t.rpcCallTimeout)
 	defer cancel()
 
@@ -289,7 +287,7 @@ func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args Se
 	if !args.IsDynamicFeeTx() && args.GasPrice == nil {
 		gasPrice, err = rpcWrapper.SuggestGasPrice(ctx)
 		if err != nil {
-			return tx, err
+			return tx, unlock, err
 		}
 	}
 
@@ -317,7 +315,7 @@ func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args Se
 			Data:     args.GetInput(),
 		})
 		if err != nil {
-			return tx, err
+			return tx, unlock, err
 		}
 		if gas < defaultGas {
 			t.log.Info("default gas will be used because estimated is lower", "estimated", gas, "default", defaultGas)
@@ -325,7 +323,7 @@ func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args Se
 		}
 	}
 	tx = t.buildTransactionWithOverrides(nonce, value, gas, gasPrice, args)
-	return tx, nil
+	return tx, unlock, nil
 }
 
 func (t *Transactor) validateAndPropagate(rpcWrapper *rpcWrapper, selectedAccount *account.SelectedExtKey, args SendTxArgs) (hash types.Hash, err error) {
@@ -333,7 +331,12 @@ func (t *Transactor) validateAndPropagate(rpcWrapper *rpcWrapper, selectedAccoun
 		return hash, err
 	}
 
-	tx, err := t.validateAndBuildTransaction(rpcWrapper, args)
+	tx, unlock, err := t.validateAndBuildTransaction(rpcWrapper, args)
+	defer func() {
+		if unlock != nil {
+			unlock(err == nil, tx.Nonce())
+		}
+	}()
 	if err != nil {
 		return hash, err
 	}