From 5934073cbc60080245b32d36e77942ee48561704 Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Fri, 30 Aug 2024 14:42:51 +0300 Subject: [PATCH] fixed missing scrs when multiple inner tx fail with the same error, caused by hash collision --- .../relayedTx/relayedTx_test.go | 86 +++++++++++++++++++ process/transaction/export_test.go | 11 ++- process/transaction/shardProcess.go | 45 +++++++--- 3 files changed, 127 insertions(+), 15 deletions(-) diff --git a/integrationTests/chainSimulator/relayedTx/relayedTx_test.go b/integrationTests/chainSimulator/relayedTx/relayedTx_test.go index 97a815549c3..fa2b02b9fe3 100644 --- a/integrationTests/chainSimulator/relayedTx/relayedTx_test.go +++ b/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") diff --git a/process/transaction/export_test.go b/process/transaction/export_test.go index 07ed7a91896..ef23d5c114f 100644 --- a/process/transaction/export_test.go +++ b/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 diff --git a/process/transaction/shardProcess.go b/process/transaction/shardProcess.go index 50fa0c94d21..6d2fbeb80f3 100644 --- a/process/transaction/shardProcess.go +++ b/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,7 +1186,14 @@ 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), @@ -1182,7 +1201,7 @@ func (txProc *txProcessor) executeFailedRelayedUserTx( SndAddr: userTx.SndAddr, PrevTxHash: originalTxHash, OriginalTxHash: originalTxHash, - ReturnMessage: []byte(errorMsg), + ReturnMessage: returnMessage, } relayerAcnt, err := txProc.getAccountFromAddress(relayerAdr)