Link inner txs to results #6529

sstanculeanu · 2024-10-10T19:09:02Z

Reasoning behind the pull request

missing link between tx results and inner txs

Proposed changes

linked results to inner txs through prevTxHash of generated scrs

Testing procedure

standard system test + full relayed v3 testing with lots of scenarios + relayed v1/v2 tests that confirm no change happened in terms of generated scrs/logs

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

was the PR targeted to the correct branch?
if this is a larger feature that probably needs more than one PR, is there a feat branch created?
if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

AdoAdoAdo · 2024-10-11T08:29:25Z

process/smartContract/processorV2/processV2.go

@@ -1412,19 +1412,19 @@ func (sc *scProcessor) isCrossShardESDTTransfer(sender []byte, receiver []byte,
 func (sc *scProcessor) getOriginalTxHashIfIntraShardRelayedSCR(
 	tx data.TransactionHandler,
 	txHash []byte,


which txHash is this, is it the hash of the tx given as argument?

this is the hash of the initial SCR, which is the tx param

AdoAdoAdo · 2024-10-11T08:30:39Z

process/smartContract/processorV2/process_test.go

@@ -4083,21 +4083,24 @@ func TestProcessGetOriginalTxHashForRelayedIntraShard(t *testing.T) {
 	scr := &smartContractResult.SmartContractResult{Value: big.NewInt(1), SndAddr: bytes.Repeat([]byte{1}, 32)}
 	scrHash := []byte("hash")


is this a bad naming for the relay tx hash? or this really represents the scr hash?

it is the hash of the provided scr

AdoAdoAdo · 2024-10-11T08:31:25Z

process/smartContract/processorV2/process_test.go

 	assert.Equal(t, scrHash, logHash)
+	assert.Equal(t, scrHash, originalTxHash)


I think this check is not correct if the scrHash is indeed the hash of the scr.
Is this just a bad naming?
The originalTxHash should be the relay tx hash, or a transaction hash, not a scr hash.

the check is correct, the naming of the returned value is not perfect.. renamed

github-actions · 2024-10-11T12:15:00Z

📊 MultiversX Automated Test Report: View Report

🔄 Build Details:

mx-chain-go Commit Hash: 072a375241e5b6d3803487cd05f06d850bcc96a2
Current Branch: inner_hash_on_scr_prev
mx-chain-go Target Branch: rc/v1.7.next1
mx-chain-simulator-go Target Branch: rc/v1.7.next1
mx-chain-testing-suite Target Branch: rc/v1.7.next1

🚀 Environment Variables:

TIMESTAMP: 11102024-121458
PYTEST_EXIT_CODE: 1
🎉 MultiversX CI/CD Workflow Complete!

sstanculeanu · 2024-10-11T14:43:23Z

Run Tests:
mx-chain-simulator-go: link_inner_tx_to_results
mx-chain-testing-suite: fix-failed-test

raduchis · 2024-10-11T12:22:53Z

go.mod

@@ -15,9 +15,9 @@ require (
 	github.com/klauspost/cpuid/v2 v2.2.5
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/multiversx/mx-chain-communication-go v1.1.0
-	github.com/multiversx/mx-chain-core-go v1.2.22
+	github.com/multiversx/mx-chain-core-go v1.2.23-0.20241010094929-2bbea4371d73


do not forget the proper release

raduchis · 2024-10-11T12:39:48Z

process/smartContract/processorV2/process_test.go

@@ -4083,21 +4083,25 @@ func TestProcessGetOriginalTxHashForRelayedIntraShard(t *testing.T) {
 	scr := &smartContractResult.SmartContractResult{Value: big.NewInt(1), SndAddr: bytes.Repeat([]byte{1}, 32)}
 	scrHash := []byte("hash")

-	logHash, isRelayed := sc.getOriginalTxHashIfIntraShardRelayedSCR(scr, scrHash)
+	logHash, originalTxHash, isRelayed := sc.getOriginalTxHashIfIntraShardRelayedSCR(scr, scrHash)


this is actually a scrHash, not a logHash, right?

might be either a scr hash or the relayed tx hash, renamed to hashForLogSave

raduchis · 2024-10-11T12:51:53Z

process/smartContract/processorV2/processV2.go

+	// even for intra-shard sc calls
+	if !bytes.Equal(relayedSCR.PrevTxHash, relayedSCR.OriginalTxHash) {
+		return txHash, relayedSCR.OriginalTxHash, isRelayed
+


empty line can be removed

raduchis · 2024-10-11T15:15:32Z

node/external/transactionAPI/apiTransactionProcessor.go

+
+	for _, innerTx := range tx.InnerTransactions {
+		innerTx.Epoch = tx.Epoch
+		innerTx.Round = tx.Round


no need for the miniblockHash here?

added, forgot from the last review

raduchis · 2024-10-11T15:30:09Z

process/transaction/shardProcess.go

@@ -789,6 +789,20 @@ func (txProc *txProcessor) processInnerTx(

 	txFee := txProc.computeInnerTxFee(innerTx)

+	prevTxHash, err := txProc.getPrevTxHashForInnerTxOfRelayedV3(originalTxHash, innerTx)


this is added without any flags and it might not be backwards compatible if an error occurs. Previously no error was returned here.

the flag is inside getPrevTxHashForInnerTxOfRelayedV3.. if the flag is not active yet, it will simply return originalTxHash and nil, as it is currently on devnet.. thus it should work as intended

AdoAdoAdo · 2024-10-14T05:57:12Z

process/smartContract/processorV2/processV2.go

@@ -1412,16 +1412,17 @@ func (sc *scProcessor) isCrossShardESDTTransfer(sender []byte, receiver []byte,
 func (sc *scProcessor) getOriginalTxHashIfIntraShardRelayedSCR(
 	tx data.TransactionHandler,
 	txHash []byte,
-) ([]byte, []byte, bool) {


todo: use a struct for the return values

github-actions · 2024-10-17T09:24:21Z

📊 MultiversX Automated Test Report: View Report

🔄 Build Details:

mx-chain-go Commit Hash: 5389337a3ecede76cf8072be0b57f7f1cf9af0d5
Current Branch: inner_hash_on_scr_prev
mx-chain-go Target Branch: rc/v1.7.next1
mx-chain-simulator-go Target Branch: rc/v1.7.next1
mx-chain-testing-suite Target Branch: rc/v1.7.next1

🚀 Environment Variables:

TIMESTAMP: 17102024-092418
PYTEST_EXIT_CODE: 1
🎉 MultiversX CI/CD Workflow Complete!

prev hash of scrs is now inner tx hash in order to link them

cf76a45

sstanculeanu added type:bug Something isn't working area:processing impact:medium interested:explorer interested:api-multiversx-com interested:protocol complexity:medium labels Oct 10, 2024

sstanculeanu self-assigned this Oct 10, 2024

sstanculeanu added 4 commits October 10, 2024 22:44

check for duplicated inner txs

e9c5ae2

fix tests

c5d7285

set prevTxHash on scr + integration tests update

06fb526

improve tests

11c3a2d

AdoAdoAdo self-requested a review October 11, 2024 10:04

added scr for failed inner tx sc call

072a375

AdoAdoAdo reviewed Oct 11, 2024

View reviewed changes

fix after review

be94f0e

multiversx deleted a comment from github-actions bot Oct 11, 2024

AdoAdoAdo previously approved these changes Oct 11, 2024

View reviewed changes

raduchis reviewed Oct 11, 2024

View reviewed changes

fix after review

0585d97

sstanculeanu dismissed AdoAdoAdo’s stale review via 0585d97 October 11, 2024 15:43

sstanculeanu added 2 commits October 11, 2024 20:30

updated indexer

5aa4fb6

fix condition from sc processor to apply for intra shard only

9894635

multiversx deleted a comment from github-actions bot Oct 12, 2024

AdoAdoAdo previously approved these changes Oct 14, 2024

View reviewed changes

andreibancioiu mentioned this pull request Oct 14, 2024

Handle new response of relayed V3 multiversx/mx-sdk-js-network-providers#72

Open

raduchis previously approved these changes Oct 14, 2024

View reviewed changes

fix long tests

0543e9b

sstanculeanu dismissed stale reviews from raduchis and AdoAdoAdo via 0543e9b October 15, 2024 10:27

This was referenced Oct 17, 2024

fix missing log from final relayed tx result #6520

Closed

Link inner tx to results #6527

Closed

sstanculeanu added 2 commits October 17, 2024 11:40

improve tests + save non executable log of relayed v3 to inner tx hash

6280d4c

add inner logs into the final tx

5389337

multiversx deleted a comment from github-actions bot Oct 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link inner txs to results #6529

Link inner txs to results #6529

sstanculeanu commented Oct 10, 2024

AdoAdoAdo Oct 11, 2024

sstanculeanu Oct 11, 2024

AdoAdoAdo Oct 11, 2024

sstanculeanu Oct 11, 2024 •

edited

Loading

AdoAdoAdo Oct 11, 2024

sstanculeanu Oct 11, 2024

github-actions bot commented Oct 11, 2024

sstanculeanu commented Oct 11, 2024

raduchis Oct 11, 2024

raduchis Oct 11, 2024

sstanculeanu Oct 11, 2024

raduchis Oct 13, 2024

raduchis Oct 11, 2024

sstanculeanu Oct 11, 2024

raduchis Oct 13, 2024

raduchis Oct 11, 2024

sstanculeanu Oct 11, 2024

raduchis Oct 13, 2024

raduchis Oct 11, 2024

sstanculeanu Oct 11, 2024

raduchis Oct 13, 2024

AdoAdoAdo Oct 14, 2024

github-actions bot commented Oct 17, 2024

		@@ -4083,21 +4083,24 @@ func TestProcessGetOriginalTxHashForRelayedIntraShard(t *testing.T) {
		scr := &smartContractResult.SmartContractResult{Value: big.NewInt(1), SndAddr: bytes.Repeat([]byte{1}, 32)}
		scrHash := []byte("hash")

		assert.Equal(t, scrHash, logHash)
		assert.Equal(t, scrHash, originalTxHash)

		@@ -789,6 +789,20 @@ func (txProc *txProcessor) processInnerTx(

		txFee := txProc.computeInnerTxFee(innerTx)

		prevTxHash, err := txProc.getPrevTxHashForInnerTxOfRelayedV3(originalTxHash, innerTx)

Link inner txs to results #6529

Are you sure you want to change the base?

Link inner txs to results #6529

Conversation

sstanculeanu commented Oct 10, 2024

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sstanculeanu Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 11, 2024

sstanculeanu commented Oct 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2024

sstanculeanu Oct 11, 2024 •

edited

Loading