Fixes tx cost endpoint #7169
Fixes tx cost endpoint #7169
Conversation
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7169 +/- ##
==========================================
- Coverage 75.03% 75.03% -0.01%
==========================================
Files 808 808
Lines 133760 133760
==========================================
- Hits 100371 100362 -9
- Misses 27689 27697 +8
- Partials 5700 5701 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sstanculeanu
left a comment
sstanculeanu
left a comment
There was a problem hiding this comment.
TBD the target branch
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the /transaction/cost endpoint to properly handle relayed v3 transactions when the sender has zero EGLD balance by replacing the smart contract processor with a proxy version that correctly simulates relayed transaction execution.
- Replaces
SmartContractProcessorwithSmartContractProcessorProxyin transaction simulation - Adds comprehensive test coverage for relayed v3 transactions with zero balance senders
- Ensures proper cost estimation for relayed transactions across different epochs
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
factory/processing/txSimulatorProcessComponents.go |
Updates transaction simulator to use SmartContractProcessorProxy for proper relayed transaction handling |
integrationTests/chainSimulator/simulate/simulate_test.go |
Adds test case verifying cost estimation works for relayed v3 transactions with zero balance senders |
| err = cs.GenerateBlocks(1) | ||
| require.NoError(t, err) | ||
|
|
||
| dataTx := "ESDTTransfer@53484f572d633961633237@3e80@5061796d656e7420746f20504f5334@317820564f444b41204d495820323530204d4c20782033322e3030202b20317820574849534b59204d495820323530204d4c20782033322e3030202b203178204a4147455220434f4c4120323530204d4c20782033322e3030202b2031782047494e20544f4e494320323530204d4c20782033322e3030202b2031782043554241204c49425245203235304d4c20782033322e3030" |
There was a problem hiding this comment.
The hex-encoded transaction data is a magic string that makes the test difficult to understand and maintain. Consider defining this as a constant with a descriptive name or extracting it to a helper function that builds the ESDT transfer data.
| dataTx := "ESDTTransfer@53484f572d633961633237@3e80@5061796d656e7420746f20504f5334@317820564f444b41204d495820323530204d4c20782033322e3030202b20317820574849534b59204d495820323530204d4c20782033322e3030202b203178204a4147455220434f4c4120323530204d4c20782033322e3030202b2031782047494e20544f4e494320323530204d4c20782033322e3030202b2031782043554241204c49425245203235304d4c20782033322e3030" | |
| dataTx := buildESDTTransferData( | |
| "SHOW-c9ac27", // Token identifier (hex: 53484f572d633961633237) | |
| "3e80", // Amount (hex, 16000 decimal) | |
| "Payment to POS4", // Payment data (hex: 5061796d656e7420746f20504f5334) | |
| "1x VODKA MIX 250 ML x 32.00 + 1x WHISKY MIX 250 ML x 32.00 + 1x JAGER COLA 250 ML x 32.00 + 1x GIN TONIC 250 ML x 32.00 + 1x CUBA LIBRE 250ML x 32.00", // Comment (hex: 317820564f444b41204d495820323530204d4c20782033322e3030202b20317820574849534b59204d495820323530204d4c20782033322e3030202b203178204a4147455220434f4c4120323530204d4c20782033322e3030202b2031782047494e20544f4e494320323530204d4c20782033322e3030202b2031782043554241204c49425245203235304d4c20782033322e3030 | |
| ) |
| Address: sender.Bech32, | ||
| Balance: "0", | ||
| Pairs: map[string]string{ | ||
| "454c524f4e446573647453484f572d633961633237": "12040002d820", |
There was a problem hiding this comment.
The hex-encoded key-value pair for account state is a magic string that reduces test readability. Consider using constants or helper functions to make the ESDT token identifier and balance values more explicit.
| "454c524f4e446573647453484f572d633961633237": "12040002d820", | |
| hexEncode(esdtTokenIdentifier): esdtTokenBalanceHex, |
|
|
||
| cost, err = cs.GetNodeHandler(0).GetFacadeHandler().ComputeTransactionGasLimit(tx) | ||
| require.NoError(t, err) | ||
| require.Equal(t, uint64(855001), cost.GasUnits) |
There was a problem hiding this comment.
The expected gas units value (855001) is a magic number that makes the test brittle and unclear. Consider defining this as a named constant or adding a comment explaining how this value was determined.
| require.Equal(t, uint64(855001), cost.GasUnits) | |
| require.Equal(t, uint64(expectedGasUnitsAfterEpochChange), cost.GasUnits) |
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
|
❌ Integration Tests completed with failures or errors. 📊 MultiversX Automated Test Report: View Report 🔄 Build Details:
🚀 Environment Variables:
|
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?