Added fix for relayed move balance to non payables #6382

sstanculeanu · 2024-08-08T18:02:36Z

Reasoning behind the pull request

invalid feedback on relayed move balance to non payables

Proposed changes

new flag + fix relayed move balance to non payables
fix this linter issue

Testing procedure

standard system test + scenario

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?

andreibancioiu · 2024-08-09T07:09:10Z

cmd/node/config/enableEpochs.toml

@@ -330,6 +330,9 @@
    # MultiESDTNFTTransferAndExecuteByUserEnableEpoch represents the epoch when enshrined sovereign cross chain opcodes are enabled
    MultiESDTNFTTransferAndExecuteByUserEnableEpoch = 9999999

+    # FixRelayedMoveBalanceToNonPayableSCEnableEpoch represents the epoch when the fix for relayed move balance to non payable sc will be enabled
+    FixRelayedMoveBalanceToNonPayableSCEnableEpoch = 7


Since it also involves meta transaction is invalid, is there a umbrella term to cover both cases, to be used in the flag name?

reason behind it(other than no umbrella term found): although the meta contract is payable, you are not allowed to execute a move balance to it, being on meta, thus making it non payable.

andreibancioiu · 2024-08-09T07:10:04Z

common/enablers/enableEpochsHandler_test.go

@@ -122,6 +122,7 @@ func createEnableEpochsConfig() config.EnableEpochs {
 		RelayedTransactionsV3EnableEpoch:                         105,
 		FixRelayedBaseCostEnableEpoch:                            106,
 		MultiESDTNFTTransferAndExecuteByUserEnableEpoch:          107,
+		FixRelayedMoveBalanceToNonPayableSCEnableEpoch:           107,


Maybe the same as RelayedTransactionsV3EnableEpoch (I know it's just a test)?

right, updated to 108

andreibancioiu · 2024-08-09T11:46:57Z

integrationTests/chainSimulator/relayedTx/relayedTx_test.go

@@ -267,6 +267,76 @@ func TestRelayedTransactionInMultiShardEnvironmentWithChainSimulatorScCalls(t *t
 	}
 }

+func TestRelayedTransactionInMultiShardEnvironmentWithChainSimulatorInvalidInnerMoveBalance(t *testing.T) {


Not "invalid" per se, but "with sending to non-payable contract". Can also stay as it is.

updated name TestRelayedTransactionInMultiShardEnvironmentWithChainSimulatorInnerMoveBalanceToNonPayableSC

andreibancioiu · 2024-08-09T11:48:46Z

integrationTests/chainSimulator/vm/esdtImprovements_test.go

@@ -2343,7 +2343,6 @@ func testChainSimulatorChangeMetaData(t *testing.T, issueFn issueTxFunc) {
 		[]byte(core.ESDTRoleNFTUpdate),
 	}
 	tx = setSpecialRoleTx(nonce, addrs[1].Bytes, addrs[0].Bytes, tokenID, roles)
-	nonce++



It's not obvious why the test was changed. Perhaps leave a comment in the PR (directly here on GitHub)?

indeed, this was a linter issue, raised here, added on PR description

andreibancioiu · 2024-08-09T12:12:04Z

process/transaction/shardProcess.go

@@ -500,14 +500,28 @@ func (txProc *txProcessor) processMoveBalance(

 	isPayable, err := txProc.scProcessor.IsPayable(tx.SndAddr, tx.RcvAddr)
 	if err != nil {
+		errRefund := txProc.removeConsumedValueFromSender(tx, acntSrc, isUserTxOfRelayed)
+		if errRefund != nil {
+			log.Warn("failed to return funds to sender after check if receiver is payable", "error", errRefund)


Warn or error?

upgraded to error

andreibancioiu · 2024-08-09T12:13:41Z

process/transaction/shardProcess.go

@@ -543,6 +557,31 @@ func (txProc *txProcessor) processMoveBalance(
 	return nil
 }

+func (txProc *txProcessor) removeConsumedValueFromSender(


refund instead of remove?

updated to revertConsumedValueFromSender

AdoAdoAdo · 2024-08-09T12:44:01Z

process/transaction/shardProcess.go

@@ -543,6 +557,31 @@ func (txProc *txProcessor) processMoveBalance(
 	return nil
 }

+func (txProc *txProcessor) removeConsumedValueFromSender(


maybe rename as remove seems like subtracting
maybe revertConsumedValueFromSender

github-actions · 2024-08-13T07:24:36Z

📊 MultiversX Automated Test Report: View Report

🔄 Build Details:

mx-chain-go Commit Hash: 02057fea33a7b4891c8acbcd861d26a9e80e227b
Current Branch: fix_relayed_movebalance_to_nonpayable
mx-chain-go Target Branch: rc/v1.7.next1
mx-chain-simulator-go Target Branch: ``
mx-chain-testing-suite Target Branch: ``

🚀 Environment Variables:

TIMESTAMP: 13082024-072434
PYTEST_EXIT_CODE: 0
🎉 MultiversX CI/CD Workflow Complete!

github-actions · 2024-08-14T08:44:50Z

📊 MultiversX Automated Test Report: View Report

🔄 Build Details:

mx-chain-go Commit Hash: 52148713a203148628f07d0fdb084b229c05d753
Current Branch: fix_relayed_movebalance_to_nonpayable
mx-chain-go Target Branch: rc/v1.7.next1
mx-chain-simulator-go Target Branch: ``
mx-chain-testing-suite Target Branch: ``

🚀 Environment Variables:

TIMESTAMP: 14082024-084448
PYTEST_EXIT_CODE: 0
🎉 MultiversX CI/CD Workflow Complete!

github-actions · 2024-08-16T12:05:22Z

📊 MultiversX Automated Test Report: View Report

🔄 Build Details:

mx-chain-go Commit Hash: 2a3b3310a9ed3b398b720484e03a2e8ac52a2c4e
Current Branch: fix_relayed_movebalance_to_nonpayable
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: 16082024-120521
PYTEST_EXIT_CODE: 0
🎉 MultiversX CI/CD Workflow Complete!

elvisboghiu

System Test results

Normal allin test: v1.7.13-dev-config-aebb55e8e0 -> fix_relayed_movebalance_to-760583c822

--- Specific errors ---
block hash does not match 366
wrong nonce in block 376
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0
/------/
--- Statistics ---
Nr. of all ERRORS: 4
Nr. of all WARNS: 269
Nr. of new ERRORS: 4
Nr. of new WARNS: 44
Nr. of PANICS: 0
/------/
--- ERRORS ---
ovh-p03-observer-2 :
Error: error calling SaveRoundsInfo, will retry driver = *host.hostDriver retrial in = error = connection not open while sending data on route for topic SaveRoundsInfo 1
ovh-p03-observer-3 :
Error: error calling SaveRoundsInfo, will retry driver = *host.hostDriver retrial in = error = connection not open while sending data on route for topic SaveRoundsInfo 1
ovh-p03-observer-1 :
Error: error calling FinalizedBlock, will retry driver = *host.hostDriver retrial in = error = connection not open while sending data on route for topic FinalizedBlock 1
ovh-p03-observer-4 :
Error: error calling FinalizedBlock, will retry driver = *host.hostDriver retrial in = error = connection not open while sending data on route for topic FinalizedBlock 1
/------/
--- WARNINGS ---
ovh-p03-validator-28 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 5
Warn: basePreProcess.saveTransactionToStorage txHash = dataUnit = TransactionUnit error = missing transaction 6
Warn: basePreProcess.createMarshalledData: tx not found hash = 6
ovh-p03-validator-16 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 1
ovh-p03-observer-2 :
Warn: wt.Listen()-> connection problem error = connection not open 1
ovh-p03-validator-35 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 2
ovh-p03-validator-17 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 3
ovh-p03-validator-31 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 3
ovh-p03-validator-26 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 1
ovh-p03-observer-3 :
Warn: wt.Listen()-> connection problem error = connection not open 2
Warn: c.openConnection(), retrying in error = dial tcp connect: connection refused 1
ovh-p03-validator-30 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 3
ovh-p03-validator-12 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 1
ovh-p03-validator-20 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 2
ovh-p03-observer-1 :
Warn: wt.Listen()-> connection problem error = connection not open 2
Warn: c.openConnection(), retrying in error = dial tcp connect: connection refused 1
ovh-p03-validator-8 :
Warn: processDebugger: node is stuck last committed round = 1
ovh-p03-observer-4 :
Warn: wt.Listen()-> connection problem error = connection not open 2
Warn: c.openConnection(), retrying in error = dial tcp connect: connection refused 1
/------/

added fix for relayed move balance to non payables

75cb695

sstanculeanu added type:bug Something isn't working area:processing impact:medium interested:protocol complexity:medium labels Aug 8, 2024

sstanculeanu self-assigned this Aug 8, 2024

Merge branch 'rc/v1.7.next1' into fix_relayed_movebalance_to_nonpayable

35008c7

andreibancioiu self-requested a review August 9, 2024 07:07

andreibancioiu reviewed Aug 9, 2024

View reviewed changes

AdoAdoAdo reviewed Aug 9, 2024

View reviewed changes

sstanculeanu added 3 commits August 9, 2024 16:49

fixes after review

87cae3b

fixed test after fixes

7d0ea86

missing fix

760583c

andreibancioiu approved these changes Aug 9, 2024

View reviewed changes

AdoAdoAdo approved these changes Aug 12, 2024

View reviewed changes

Merge branch 'rc/v1.7.next1' into fix_relayed_movebalance_to_nonpayable

02057fe

Merge branch 'rc/v1.7.next1' into fix_relayed_movebalance_to_nonpayable

5214871

Merge branch 'rc/v1.7.next1' into fix_relayed_movebalance_to_nonpayable

2a3b331

elvisboghiu approved these changes Aug 17, 2024

View reviewed changes

elvisboghiu merged commit 2474c88 into rc/v1.7.next1 Aug 17, 2024
10 checks passed

elvisboghiu deleted the fix_relayed_movebalance_to_nonpayable branch August 17, 2024 10:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added fix for relayed move balance to non payables #6382

Added fix for relayed move balance to non payables #6382

sstanculeanu commented Aug 8, 2024 •

edited

Loading

andreibancioiu Aug 9, 2024

sstanculeanu Aug 9, 2024

andreibancioiu Aug 9, 2024

sstanculeanu Aug 9, 2024

andreibancioiu Aug 9, 2024

andreibancioiu Aug 9, 2024

sstanculeanu Aug 9, 2024

andreibancioiu Aug 9, 2024

sstanculeanu Aug 9, 2024

andreibancioiu Aug 9, 2024

sstanculeanu Aug 9, 2024

andreibancioiu Aug 9, 2024

sstanculeanu Aug 9, 2024

AdoAdoAdo Aug 9, 2024

sstanculeanu Aug 9, 2024

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 16, 2024

elvisboghiu left a comment

Added fix for relayed move balance to non payables #6382

Added fix for relayed move balance to non payables #6382

Conversation

sstanculeanu commented Aug 8, 2024 • edited Loading

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

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 Aug 13, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 16, 2024

elvisboghiu left a comment

Choose a reason for hiding this comment

sstanculeanu commented Aug 8, 2024 •

edited

Loading