Fix how gasUsed is calculated on move balance transaction #2349
Conversation
| @@ -98,6 +100,8 @@ func (cm *commonProcessor) buildTransaction( | |||
| header data.HeaderHandler, | |||
| txStatus string, | |||
| ) *Transaction { | |||
| gasUsed := cm.minGasLimit + uint64(len(tx.Data))*cm.gasPerDataByte | |||
There was a problem hiding this comment.
For invalid transactions, after the Mainnet upgrade, this will not be correct anymore. Perhaps use the soft fork swtich here as well (PenalizedTooMuchGasEnableEpoch - @sasurobert, @SebastianMarian is this the one?).
After the Mainnet upgrade, for invalid transactions, gasUsed := gasLimit.
There was a problem hiding this comment.
for invalid transactions gasUsed will be deduced from the receipt.
| @@ -57,6 +59,10 @@ func createMockElasticProcessorArgs() ArgElasticProcessor { | |||
| blockIndex: {}, txIndex: {}, miniblocksIndex: {}, tpsIndex: {}, validatorsIndex: {}, roundIndex: {}, accountsIndex: {}, ratingIndex: {}, accountsHistoryIndex: {}, | |||
| }, | |||
| AccountsDB: &mock.AccountsStub{}, | |||
| FeeConfig: &config.FeeSettings{ | |||
| MinGasLimit: "10", | |||
There was a problem hiding this comment.
We can use "real" values in tests as well (optional).
core/indexer/common_test.go
Outdated
| vmcommon "github.com/ElrondNetwork/elrond-vm-common" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func createCommonProcessor() commonProcessor { | ||
| func createCommonProcessor(minGasLimit, gasPerDataByte uint64) commonProcessor { |
There was a problem hiding this comment.
minGasLimit uint64, gasPerDataByte uint64 keep our standard
core/indexer/processTransactions.go
Outdated
| @@ -140,6 +136,22 @@ func (tdp *txDatabaseProcessor) prepareTransactionsForDatabase( | |||
| return append(convertMapTxsToSlice(transactions), rewardsTxs...), alteredAddresses | |||
| } | |||
|
|
|||
| func getGasUsedFromReceipt(rec *receipt.Receipt, tx *Transaction) uint64 { | |||
| if rec.Data != nil && string(rec.Data) == transactionProcess.RefundGasMessage { | |||
There was a problem hiding this comment.
Do we need rec.Data != nil here?
There was a problem hiding this comment.
I prefer to have this check just to be sure the cast to string never panics.
core/indexer/processTransactions.go
Outdated
| @@ -19,6 +19,7 @@ import ( | |||
| "github.com/ElrondNetwork/elrond-go/hashing" | |||
| "github.com/ElrondNetwork/elrond-go/marshal" | |||
| "github.com/ElrondNetwork/elrond-go/process" | |||
| transactionProcess "github.com/ElrondNetwork/elrond-go/process/transaction" | |||
There was a problem hiding this comment.
processTransaction?
| return gasUsed.Uint64() | ||
| } | ||
|
|
||
| gasUsed := big.NewInt(0) |
There was a problem hiding this comment.
Maybe add a comment, that in this case the receipt contains the cost (the fee).
| @@ -13,6 +13,7 @@ import ( | |||
| "github.com/ElrondNetwork/elrond-go/data/rewardTx" | |||
| "github.com/ElrondNetwork/elrond-go/data/smartContractResult" | |||
| "github.com/ElrondNetwork/elrond-go/data/transaction" | |||
| transactionProcess "github.com/ElrondNetwork/elrond-go/process/transaction" | |||
There was a problem hiding this comment.
processTransaction.
| GasLimit: gasLimit, | ||
| } | ||
|
|
||
| expectedGasUsed := big.NewInt(0).SetUint64(gasPrice) |
There was a problem hiding this comment.
expectedGasUsed as a constant / a big int initialized with the expected value?
| GasLimit: gasLimit, | ||
| } | ||
|
|
||
| expectedGasUsed := big.NewInt(0) |
There was a problem hiding this comment.
expectedGasUsed as a constant / a big int initialized with the expected value - e.g. from string?
process/transaction/shardProcess.go
Outdated
| @@ -24,6 +24,10 @@ import ( | |||
| var log = logger.GetOrCreate("process/transaction") | |||
| var _ process.TransactionProcessor = (*txProcessor)(nil) | |||
|
|
|||
| // RefundGasMessage is message that is returned in data field of created receipt | |||
There was a problem hiding this comment.
// RefundGasMessage is the message returned in the data field of a receipt,
// for move balance transactions that provide more gas than needed
…used-is-calculated
…ulated' into EN-7886-fix-how-gas-used-is-calculated
…used-is-calculated
Fix how gas used is calculated for move balance transactions.
Now gasUsed for a move balance transaction is calculated with formula
minGasLimit + len(data)*gasPerDataByteTesting procedure:
run a testnet with this branch and make sure that external.toml has ElasticSearchConnector enabled
in epoch 0:generate an invalid transaction with
gasLimit= 500 000(to generate an invalid transaction value from transactionshould be greater than sender account balance.
check in elasticsearch database that gasUsed of generated transaction is equals with
50 000wait 2 epochs to pass (when you move to next step epoch number should be greater of equal with 2)
generate again an invalid transaction with gasLimit=
500 000check in elasticsearch database that gasUsed of generated transaction is equals with
500 000