Merge pull request #6426 from multiversx/fix_relayedv3_missing_scrs_o…

…n_failure

fixed missing scrs when multiple inner tx fail with the same error, caused by hash collision

integrationTests/chainSimulator/relayedTx/relayedTx_test.go

@@ -155,6 +155,92 @@ func TestRelayedTransactionInMultiShardEnvironmentWithChainSimulator(t *testing.
 	require.True(t, strings.Contains(string(result.Logs.Events[2].Data), "contract is paused"))
 }
 
+func TestRelayedTransactionInMultiShardEnvironmentWithChainSimulatorAndInvalidNonces(t *testing.T) {
+	if testing.Short() {
+		t.Skip("this is not a short test")
+	}
+
+	cs := startChainSimulator(t, alterConfigsFuncRelayedV3EarlyActivation)
+	defer cs.Close()
+
+	initialBalance := big.NewInt(0).Mul(oneEGLD, big.NewInt(30000))
+	relayer, err := cs.GenerateAndMintWalletAddress(0, initialBalance)
+	require.NoError(t, err)
+
+	sender, err := cs.GenerateAndMintWalletAddress(0, initialBalance)
+	require.NoError(t, err)
+
+	receiver, err := cs.GenerateAndMintWalletAddress(0, big.NewInt(0))
+	require.NoError(t, err)
+
+	err = cs.GenerateBlocks(1)
+	require.Nil(t, err)
+
+	// bump sender nonce to 3
+	tx0 := generateTransaction(sender.Bytes, 0, sender.Bytes, big.NewInt(0), "", minGasLimit)
+	tx1 := generateTransaction(sender.Bytes, 1, sender.Bytes, big.NewInt(0), "", minGasLimit)
+	tx2 := generateTransaction(sender.Bytes, 2, sender.Bytes, big.NewInt(0), "", minGasLimit)
+	_, err = cs.SendTxsAndGenerateBlocksTilAreExecuted([]*transaction.Transaction{tx0, tx1, tx2}, maxNumOfBlocksToGenerateWhenExecutingTx)
+	require.Nil(t, err)
+
+	// higher nonce
+	innerTx1 := generateTransaction(sender.Bytes, 10, receiver.Bytes, oneEGLD, "", minGasLimit)
+	innerTx1.RelayerAddr = relayer.Bytes
+
+	// higher nonce
+	innerTx2 := generateTransaction(sender.Bytes, 9, receiver.Bytes, oneEGLD, "", minGasLimit)
+	innerTx2.RelayerAddr = relayer.Bytes
+
+	// nonce ok
+	innerTx3 := generateTransaction(sender.Bytes, 3, receiver.Bytes, oneEGLD, "", minGasLimit)
+	innerTx3.RelayerAddr = relayer.Bytes
+
+	// higher nonce
+	innerTx4 := generateTransaction(sender.Bytes, 8, receiver.Bytes, oneEGLD, "", minGasLimit)
+	innerTx4.RelayerAddr = relayer.Bytes
+
+	// lower nonce - initial one
+	innerTx5 := generateTransaction(sender.Bytes, 3, receiver.Bytes, oneEGLD, "", minGasLimit)
+	innerTx5.RelayerAddr = relayer.Bytes
+
+	innerTxs := []*transaction.Transaction{innerTx1, innerTx2, innerTx3, innerTx4, innerTx5}
+
+	// relayer will consume first a move balance for each inner tx, then the specific gas for each inner tx
+	relayedTxGasLimit := uint64(0)
+	for _, tx := range innerTxs {
+		relayedTxGasLimit += minGasLimit + tx.GasLimit
+	}
+	relayedTx := generateTransaction(relayer.Bytes, 0, relayer.Bytes, big.NewInt(0), "", relayedTxGasLimit)
+	relayedTx.InnerTransactions = innerTxs
+
+	result, err := cs.SendTxAndGenerateBlockTilTxIsExecuted(relayedTx, maxNumOfBlocksToGenerateWhenExecutingTx)
+	require.NoError(t, err)
+
+	// 5 scrs, 4 from the failed txs + 1 with success
+	require.Equal(t, 5, len(result.SmartContractResults))
+	scrsMap := make(map[string]int, len(result.SmartContractResults))
+	for _, scr := range result.SmartContractResults {
+		if len(scr.ReturnMessage) == 0 {
+			scrsMap["success"]++
+		}
+		if strings.Contains(scr.ReturnMessage, process.ErrHigherNonceInTransaction.Error()) {
+			scrsMap[process.ErrHigherNonceInTransaction.Error()]++
+		}
+		if strings.Contains(scr.ReturnMessage, process.ErrLowerNonceInTransaction.Error()) {
+			scrsMap[process.ErrLowerNonceInTransaction.Error()]++
+		}
+	}
+	require.Equal(t, 1, scrsMap["success"])
+	require.Equal(t, 3, scrsMap[process.ErrHigherNonceInTransaction.Error()])
+	require.Equal(t, 1, scrsMap[process.ErrLowerNonceInTransaction.Error()])
+
+	// 4 log events from the failed txs
+	require.Equal(t, 4, len(result.Logs.Events))
+	for _, event := range result.Logs.Events {
+		require.Equal(t, core.SignalErrorOperation, event.Identifier)
+	}
+}
+
 func TestRelayedTransactionInMultiShardEnvironmentWithChainSimulatorScCalls(t *testing.T) {
 	if testing.Short() {
 		t.Skip("this is not a short test")

process/transaction/export_test.go

@@ -57,7 +57,13 @@ func (txProc *txProcessor) ProcessUserTx(
 	relayedNonce uint64,
 	originalTxHash []byte,
 ) (vmcommon.ReturnCode, error) {
-	return txProc.processUserTx(originalTx, userTx, relayedTxValue, relayedNonce, originalTxHash)
+	return txProc.processUserTx(
+		originalTx,
+		userTx,
+		relayedTxValue,
+		relayedNonce,
+		originalTxHash,
+		nonRelayedV3UserTxIdx)
 }
 
 // ProcessMoveBalanceCostRelayedUserTx calls the un-exported method processMoveBalanceCostRelayedUserTx
@@ -87,7 +93,8 @@ func (txProc *txProcessor) ExecuteFailedRelayedTransaction(
 		relayedNonce,
 		originalTx,
 		originalTxHash,
-		errorMsg)
+		errorMsg,
+		nonRelayedV3UserTxIdx)
 }
 
 // CheckMaxGasPrice calls the un-exported method checkMaxGasPrice

process/transaction/shardProcess.go

@@ -31,6 +31,8 @@ var _ process.TransactionProcessor = (*txProcessor)(nil)
 // for move balance transactions that provide more gas than needed
 const RefundGasMessage = "refundedGas"
 
+const nonRelayedV3UserTxIdx = -1
+
 type relayedFees struct {
 	totalFee, remainingFee, relayerFee *big.Int
 }
@@ -647,12 +649,13 @@ func (txProc *txProcessor) finishExecutionOfRelayedTx(
 			tx.Nonce,
 			tx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			nonRelayedV3UserTxIdx)
 	}
 
 	defer txProc.saveFailedLogsIfNeeded(originalTxHash)
 
-	return txProc.processUserTx(tx, userTx, tx.Value, tx.Nonce, originalTxHash)
+	return txProc.processUserTx(tx, userTx, tx.Value, tx.Nonce, originalTxHash, nonRelayedV3UserTxIdx)
 }
 
 func (txProc *txProcessor) processTxAtRelayer(
@@ -743,8 +746,8 @@ func (txProc *txProcessor) processRelayedTxV3(
 	var innerTxFee *big.Int
 	innerTxsTotalFees := big.NewInt(0)
 	executedUserTxs := make([]*transaction.Transaction, 0)
-	for _, innerTx := range innerTxs {
-		innerTxFee, innerTxRetCode, innerTxErr = txProc.processInnerTx(tx, innerTx, originalTxHash)
+	for innerTxIdx, innerTx := range innerTxs {
+		innerTxFee, innerTxRetCode, innerTxErr = txProc.processInnerTx(tx, innerTx, originalTxHash, innerTxIdx)
 		innerTxsTotalFees.Add(innerTxsTotalFees, innerTxFee)
 		if innerTxErr != nil || innerTxRetCode != vmcommon.Ok {
 			continue
@@ -781,6 +784,7 @@ func (txProc *txProcessor) processInnerTx(
 	tx *transaction.Transaction,
 	innerTx *transaction.Transaction,
 	originalTxHash []byte,
+	innerTxIdx int,
 ) (*big.Int, vmcommon.ReturnCode, error) {
 
 	txFee := txProc.computeInnerTxFee(innerTx)
@@ -794,7 +798,8 @@ func (txProc *txProcessor) processInnerTx(
 			tx.Nonce,
 			tx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			innerTxIdx)
 	}
 
 	if check.IfNil(acntSnd) {
@@ -805,7 +810,8 @@ func (txProc *txProcessor) processInnerTx(
 			tx.Nonce,
 			tx,
 			originalTxHash,
-			process.ErrRelayedTxV3SenderShardMismatch.Error())
+			process.ErrRelayedTxV3SenderShardMismatch.Error(),
+			innerTxIdx)
 	}
 
 	// TODO: remove adding and then removing the fee at the sender
@@ -818,10 +824,11 @@ func (txProc *txProcessor) processInnerTx(
 			tx.Nonce,
 			tx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			innerTxIdx)
 	}
 
-	result, err := txProc.processUserTx(tx, innerTx, tx.Value, tx.Nonce, originalTxHash)
+	result, err := txProc.processUserTx(tx, innerTx, tx.Value, tx.Nonce, originalTxHash, innerTxIdx)
 	return txFee, result, err
 }
 
@@ -1002,6 +1009,7 @@ func (txProc *txProcessor) processUserTx(
 	relayedTxValue *big.Int,
 	relayedNonce uint64,
 	originalTxHash []byte,
+	innerTxIdx int,
 ) (vmcommon.ReturnCode, error) {
 
 	relayerAdr := originalTx.SndAddr
@@ -1018,7 +1026,8 @@ func (txProc *txProcessor) processUserTx(
 			relayedNonce,
 			originalTx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			innerTxIdx)
 	}
 
 	txType, dstShardTxType := txProc.txTypeHandler.ComputeTransactionType(userTx)
@@ -1035,7 +1044,8 @@ func (txProc *txProcessor) processUserTx(
 			relayedNonce,
 			originalTx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			innerTxIdx)
 	}
 
 	scrFromTx, err := txProc.makeSCRFromUserTx(userTx, relayerAdr, relayedTxValue, originalTxHash)
@@ -1085,7 +1095,8 @@ func (txProc *txProcessor) processUserTx(
 			relayedNonce,
 			originalTx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			innerTxIdx)
 	}
 
 	if errors.Is(err, process.ErrInvalidMetaTransaction) || errors.Is(err, process.ErrAccountNotPayable) {
@@ -1096,7 +1107,8 @@ func (txProc *txProcessor) processUserTx(
 			relayedNonce,
 			originalTx,
 			originalTxHash,
-			err.Error())
+			err.Error(),
+			innerTxIdx)
 	}
 
 	if errors.Is(err, process.ErrFailedTransaction) {
@@ -1174,15 +1186,22 @@ func (txProc *txProcessor) executeFailedRelayedUserTx(
 	originalTx *transaction.Transaction,
 	originalTxHash []byte,
 	errorMsg string,
+	innerTxIdx int,
 ) error {
+
+	returnMessage := []byte(errorMsg)
+	isUserTxOfRelayedV3 := innerTxIdx != nonRelayedV3UserTxIdx
+	if isUserTxOfRelayedV3 {
+		returnMessage = []byte(fmt.Sprintf("%s while executing inner tx at index %d", errorMsg, innerTxIdx))
+	}
 	scrForRelayer := &smartContractResult.SmartContractResult{
 		Nonce:          relayedNonce,
 		Value:          big.NewInt(0).Set(relayedTxValue),
 		RcvAddr:        relayerAdr,
 		SndAddr:        userTx.SndAddr,
 		PrevTxHash:     originalTxHash,
 		OriginalTxHash: originalTxHash,
-		ReturnMessage:  []byte(errorMsg),
+		ReturnMessage:  returnMessage,
 	}
 
 	relayerAcnt, err := txProc.getAccountFromAddress(relayerAdr)