Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed transaction status for relayed v1 & v2 intra-shard transactions #426

Merged
merged 4 commits into from
Feb 15, 2024
Merged

Fixed transaction status for relayed v1 & v2 intra-shard transactions #426

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7fb0764
- fixed transaction status for relayed v1 intra-shard transactions
iulianpascalau Feb 9, 2024
8cf6446
- more fixes & tests
iulianpascalau Feb 9, 2024
a977822
- refactored solution
iulianpascalau Feb 9, 2024
2dc3755
- added support for v2 relayed transactions as well
iulianpascalau Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions process/testdata/malformedRelayedTxIntraShard.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"transaction": {
"type": "normal",
"processingTypeOnSource": "RelayedTx",
"processingTypeOnDestination": "RelayedTx",
"hash": "a4823050d2396540b17bd9290523973763142c9f655bb26cd9e33f359b6d73ad",
"receiver": "erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th",
"sender": "erd1kyaqzaprcdnv4luvanah0gfxzzsnpaygsy6pytrexll2urtd05ts9vegu7",
"gasPrice": 1000000000,
"gasLimit": 11052000,
"gasUsed": 11052000,
"data": "cmVsYXllZFR4QDdiMjI2ZTZmNmU2MzY1MjIzYTM4MzUzODJjMjI3NjYxNmM3NTY1MjIzYTMxMzAzMDMwMzAzMDMwMzAzMDMwMzAzMDMwMzAzMDMwMzAzMDMwMmMyMjcyNjU2MzY1Njk3NjY1NzIyMjNhMjI3MzU0NmY0MjY0NDM1MDQ0NWE3MzcyMmY2YTRmN2EzNzY0MzY0NTZkNDU0YjQ1NzczOTQ5Njk0MjRlNDI0OTczNjU1NDY2MmI3MjY3MzE3NDY2NTI2MzNkMjIyYzIyNzM2NTZlNjQ2NTcyMjIzYTIyNDE1NDZjNDg0Yzc2Mzk2ZjY4NmU2MzYxNmQ0MzM4Nzc2NzM5NzA2NDUxNjgzODZiNzc3MDQ3NDIzNTZhNjk0OTQ5NmYzMzQ5NDg0YjU5NGU2MTY1NDUzZDIyMmMyMjY3NjE3MzUwNzI2OTYzNjUyMjNhMzEzMDMwMzAzMDMwMzAzMDMwMzAyYzIyNjc2MTczNGM2OTZkNjk3NDIyM2EzMTMwMzAzMDMwMzAzMDMwMmMyMjYzNjg2MTY5NmU0OTQ0MjIzYTIyNTI0MTNkM2QyMjJjMjI3NjY1NzI3MzY5NmY2ZTIyM2EzMTJjMjI3MzY5Njc2ZTYxNzQ3NTcyNjUyMjNhMjI1NjRjNGY1ODQ5NTI3YTZkNDY0ZTMwNTg1MDYxNTc3MDM4NWEzMDUwNjM3NzY5MzkzOTRmNTQyYjZiNmI3Nzc0Mzg1MTU3Mzc2NjM0NzU2ZDVhNjI3ODU0NmEzNzQyNzc0NTU5NTg2MzRiMzM0NjU2NGMzNTY3NDM2YTUyNzk3MTcwMzc2ZTYzNTg2YzU1NDM3Mzc3NmY0ZTZiNmQ2MzUyNzI1MjM5NTM0MjQxM2QzZDIyN2Q=",
"notarizedAtSourceInMetaNonce":2171094,
"NotarizedAtSourceInMetaHash":"02d38e178073d1af3b77c2dc87da50f8139e00b468c436fc04f22879acf5a45f",
"notarizedAtDestinationInMetaNonce":2171094,
"notarizedAtDestinationInMetaHash":"02d38e178073d1af3b77c2dc87da50f8139e00b468c436fc04f22879acf5a45f",
"status": "success",
"receivers": [
"erd1kyaqzaprcdnv4luvanah0gfxzzsnpaygsy6pytrexll2urtd05ts9vegu7"
],
"receiversShardIDs": [
1
],
"operation": "transfer",
"initiallyPaidFee": "1152000000000000",
"fee": "1152000000000000",
"isRelayed": true,
"chainID": "D",
"version": 1,
"options": 0
}
}
82 changes: 68 additions & 14 deletions process/transactionProcessor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,19 +445,18 @@ func (tp *TransactionProcessor) computeTransactionStatus(tx *transaction.ApiTran
return transaction.TxStatusFail
}

isIntraShardRelayed, err := tp.checkIfCompletelyIntraShardRelayedTransaction(tx)
allLogs, allScrs, err := tp.gatherAllLogsAndScrs(tx)
if err != nil {
return transaction.TxStatusFail
}
if isIntraShardRelayed {
return tx.Status
log.Warn("error in TransactionProcessor.computeTransactionStatus", "error", err)
return data.TxStatusUnknown
}

allLogs, err := tp.gatherAllLogs(tx)
allLogs, err = tp.addMissingLogsOnProcessingExceptions(tx, allLogs, allScrs)
if err != nil {
log.Warn("error in TransactionProcessor.computeTransactionStatus", "error", err)
log.Warn("error in TransactionProcessor.computeTransactionStatus on addMissingLogsOnProcessingExceptions call", "error", err)
return data.TxStatusUnknown
}

if checkIfFailed(allLogs) {
return transaction.TxStatusFail
}
Expand Down Expand Up @@ -500,7 +499,50 @@ func checkIfMoveBalanceNotarized(tx *transaction.ApiTransactionResult) bool {
return true
}

func (tp *TransactionProcessor) checkIfCompletelyIntraShardRelayedTransaction(tx *transaction.ApiTransactionResult) (bool, error) {
func (tp *TransactionProcessor) addMissingLogsOnProcessingExceptions(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

tx *transaction.ApiTransactionResult,
allLogs []*transaction.ApiLogs,
allScrs []*transaction.ApiTransactionResult,
) ([]*transaction.ApiLogs, error) {
newLogs, err := tp.handleIntraShardRelayedV1MoveBalanceTransactions(tx, allScrs)
if err != nil {
return nil, err
}

allLogs = append(allLogs, newLogs...)

return allLogs, nil
}

func (tp *TransactionProcessor) handleIntraShardRelayedV1MoveBalanceTransactions(
tx *transaction.ApiTransactionResult,
allScrs []*transaction.ApiTransactionResult,
) ([]*transaction.ApiLogs, error) {
var newLogs []*transaction.ApiLogs
isIntraShardRelayedV1MoveBalanceTransaction, err := tp.isIntraShardRelayedV1MoveBalanceTransaction(tx, allScrs)
if err != nil {
return newLogs, err
}

if isIntraShardRelayedV1MoveBalanceTransaction {
newLogs = append(newLogs, &transaction.ApiLogs{
Address: tx.Sender,
Events: []*transaction.Events{
{
Address: tx.Sender,
Identifier: core.CompletedTxEventIdentifier,
},
},
})
}

return newLogs, nil
}

func (tp *TransactionProcessor) isIntraShardRelayedV1MoveBalanceTransaction(
tx *transaction.ApiTransactionResult,
allScrs []*transaction.ApiTransactionResult,
) (bool, error) {
isNotarized := tx.NotarizedAtSourceInMetaNonce > 0 && tx.NotarizedAtDestinationInMetaNonce > 0
if !isNotarized {
return false, nil
Expand All @@ -511,6 +553,16 @@ func (tp *TransactionProcessor) checkIfCompletelyIntraShardRelayedTransaction(tx
return false, nil
}

if len(allScrs) == 0 {
return false, fmt.Errorf("no smart contracts results for the given relayed transaction v1")
}

firstScr := allScrs[0]
innerIsMoveBalance := firstScr.ProcessingTypeOnSource == moveBalanceDescriptor && firstScr.ProcessingTypeOnDestination == moveBalanceDescriptor
if !innerIsMoveBalance {
return false, nil
}

senderAddress, err := tp.pubKeyConverter.Decode(tx.Sender)
if err != nil {
return false, err
Expand Down Expand Up @@ -538,10 +590,10 @@ func (tp *TransactionProcessor) checkIfCompletelyIntraShardRelayedTransaction(tx
}

isSameShardOnRelayed := tp.proc.GetShardCoordinator().SameShard(senderAddress, receiverAddress)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also be written as 2 checks / 2 calls of SameShard(): (senderWhichIsRelayer, innerTx.SndAddr) and (innerTx.SndAddr, innerTx.RcvAddr) - but all right to stay as it is, as well, it's more explicit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About relayed V1 that wrap NFT transfers: not-yet-completed transactions of such an exotic kind might reach the "is relayed completely intrashard" check, and can be mistaken as completely intrashard, and thus reach an earlier "done" status - even though they are pending.

Even so, this is an extremely exotic case (no occurrence on mainnet). I think we can skip it for now, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored the solution

isSameShardOnInner := tp.proc.GetShardCoordinator().SameShard(senderAddress, innerTx.RcvAddr) &&
tp.proc.GetShardCoordinator().SameShard(senderAddress, innerTx.SndAddr)
isSameShardOnInnerForReceiver := tp.proc.GetShardCoordinator().SameShard(senderAddress, innerTx.RcvAddr)
isSameShardOnInnerForSender := tp.proc.GetShardCoordinator().SameShard(senderAddress, innerTx.SndAddr)

return isSameShardOnRelayed && isSameShardOnInner, nil
return isSameShardOnRelayed && isSameShardOnInnerForReceiver && isSameShardOnInnerForSender, nil
}

func findIdentifierInLogs(logs []*transaction.ApiLogs, identifier string) bool {
Expand Down Expand Up @@ -573,20 +625,22 @@ func findIdentifierInSingleLog(log *transaction.ApiLogs, identifier string) bool
return false
}

func (tp *TransactionProcessor) gatherAllLogs(tx *transaction.ApiTransactionResult) ([]*transaction.ApiLogs, error) {
func (tp *TransactionProcessor) gatherAllLogsAndScrs(tx *transaction.ApiTransactionResult) ([]*transaction.ApiLogs, []*transaction.ApiTransactionResult, error) {
const withResults = true
allLogs := []*transaction.ApiLogs{tx.Logs}
allScrs := make([]*transaction.ApiTransactionResult, 0)

for _, scrFromTx := range tx.SmartContractResults {
scr, err := tp.GetTransaction(scrFromTx.Hash, withResults)
if err != nil {
return nil, fmt.Errorf("%w for scr hash %s", err, scrFromTx.Hash)
return nil, nil, fmt.Errorf("%w for scr hash %s", err, scrFromTx.Hash)
}

allLogs = append(allLogs, scr.Logs)
allScrs = append(allScrs, scr)
}

return allLogs, nil
return allLogs, allScrs, nil
}

func (tp *TransactionProcessor) getTxFromObservers(txHash string, reqType requestType, withResults bool) (*transaction.ApiTransactionResult, error) {
Expand Down
42 changes: 37 additions & 5 deletions process/transactionProcessor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1925,6 +1925,18 @@ func TestTransactionProcessor_computeTransactionStatus(t *testing.T) {
status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusSuccess, status)
})
t.Run("ok relayed sc call function balance intra shard transaction still pending", func(t *testing.T) {
t.Parallel()

testData := loadJsonIntoTxAndScrs(t, "./testdata/finishedOKRelayedTxIntraShard.json")
tp := createTestProcessorFromScenarioData(testData)

testData.SCRs[0].ProcessingTypeOnSource = "SCInvoking"
testData.SCRs[0].ProcessingTypeOnDestination = "SCInvoking"

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusPending, status)
})
t.Run("ok relayed move balance cross shard transaction", func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1974,7 +1986,7 @@ func TestTransactionProcessor_computeTransactionStatus(t *testing.T) {
testData.Transaction.Sender = "not a sender"

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusFail, status)
require.Equal(t, data.TxStatusUnknown, status)
})
t.Run("malformed relayed v1 inner transaction - wrong receiver", func(t *testing.T) {
t.Parallel()
Expand All @@ -1985,7 +1997,7 @@ func TestTransactionProcessor_computeTransactionStatus(t *testing.T) {
testData.Transaction.Receiver = "not a sender"

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusFail, status)
require.Equal(t, data.TxStatusUnknown, status)
})
t.Run("malformed relayed v1 - relayed v1 marker on wrong position", func(t *testing.T) {
t.Parallel()
Expand All @@ -1996,7 +2008,7 @@ func TestTransactionProcessor_computeTransactionStatus(t *testing.T) {
testData.Transaction.Data = append([]byte("A"), testData.Transaction.Data...)

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusFail, status)
require.Equal(t, data.TxStatusUnknown, status)
})
t.Run("malformed relayed v1 - not a hex character after the marker", func(t *testing.T) {
t.Parallel()
Expand All @@ -2007,7 +2019,7 @@ func TestTransactionProcessor_computeTransactionStatus(t *testing.T) {
testData.Transaction.Data[45] = byte('T')

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusFail, status)
require.Equal(t, data.TxStatusUnknown, status)
})
t.Run("malformed relayed v1 - marshaller will fail", func(t *testing.T) {
t.Parallel()
Expand All @@ -2018,7 +2030,27 @@ func TestTransactionProcessor_computeTransactionStatus(t *testing.T) {
testData.Transaction.Data = append(testData.Transaction.Data, []byte("aaaaaa")...)

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, transaction.TxStatusFail, status)
require.Equal(t, data.TxStatusUnknown, status)
})
t.Run("malformed relayed v1 - missing scrs", func(t *testing.T) {
t.Parallel()

testData := loadJsonIntoTxAndScrs(t, "./testdata/finishedOKRelayedTxIntraShard.json")
tp := createTestProcessorFromScenarioData(testData)

testData.SCRs = nil

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, data.TxStatusUnknown, status)
})
t.Run("malformed relayed v1 - no scr generated", func(t *testing.T) {
t.Parallel()

testData := loadJsonIntoTxAndScrs(t, "./testdata/malformedRelayedTxIntraShard.json")
tp := createTestProcessorFromScenarioData(testData)

status := tp.ComputeTransactionStatus(testData.Transaction, withResults)
require.Equal(t, data.TxStatusUnknown, status)
})
})
}
Expand Down
Loading