-
Notifications
You must be signed in to change notification settings - Fork 204
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.
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
fix gas used and fee relayed v3 #6541
Conversation
# Conflicts: # go.mod # go.sum
process/economics/economicsData.go
Outdated
@@ -580,6 +591,16 @@ func (ed *economicsData) ComputeGasUsedAndFeeBasedOnRefundValue(tx data.Transact | |||
return ed.ComputeGasUsedAndFeeBasedOnRefundValueInEpoch(tx, refundValue, currentEpoch) | |||
} | |||
|
|||
// ComputeGasUnitsFromRefundValue will compute the gas unit based on the refund value | |||
func (ed *economicsData) ComputeGasUnitsFromRefundValue(tx data.TransactionWithFeeHandler, refundValue *big.Int) uint64 { | |||
currentEpoch := ed.enableEpochsHandler.GetCurrentEpoch() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
epoch should be the one of tx here as well, so perhaps get it as parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
ComputeGasUnitForRelayedV3Called func(tx *transaction.ApiTransactionResult) uint64 | ||
} | ||
|
||
func (stub *FeeComputerStub) ComputeGasUnitForRelayedV3(tx *transaction.ApiTransactionResult) uint64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
@@ -258,6 +258,22 @@ func (tep *transactionsFeeProcessor) prepareRelayedTxV3WithResults(txHashHex str | |||
refundsValue.Add(refundsValue, scr.Value) | |||
} | |||
|
|||
isRelayedV3 := len(txWithResults.GetTxHandler().GetUserTransactions()) > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should also check if it has refunds here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case no, the refunds are in the Smart Contract Results map.
@@ -52,6 +52,16 @@ type EconomicsHandlerStub struct { | |||
ComputeMoveBalanceFeeInEpochCalled func(tx data.TransactionWithFeeHandler, epoch uint32) *big.Int | |||
} | |||
|
|||
// ComputeRelayedTxV3GasUnits - | |||
func (e *EconomicsHandlerStub) ComputeRelayedTxV3GasUnits(_ data.TransactionWithFeeHandler, _ uint32) uint64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can adapt the new definitions to call user defined functions, just like the other methods in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -52,6 +52,16 @@ type EconomicsHandlerMock struct { | |||
SetTxTypeHandlerCalled func(txTypeHandler process.TxTypeHandler) error | |||
} | |||
|
|||
// ComputeRelayedTxV3GasUnits - | |||
func (ehm *EconomicsHandlerMock) ComputeRelayedTxV3GasUnits(_ data.TransactionWithFeeHandler, _ uint32) uint64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can adapt the new definitions to call user defined functions, just like the other methods in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -258,6 +258,22 @@ func (tep *transactionsFeeProcessor) prepareRelayedTxV3WithResults(txHashHex str | |||
refundsValue.Add(refundsValue, scr.Value) | |||
} | |||
|
|||
isRelayedV3 := len(txWithResults.GetTxHandler().GetUserTransactions()) > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a similar check is already done before calling the method so it is already targeted for relayed v3
maybe the ckeck can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored
gasUnits := tep.txFeeCalculator.ComputeGasUnitsFromRefundValue(txFromStorage, scr.Value, epoch) | ||
|
||
scrHandler.GetFeeInfo().SetForRelayed() | ||
scrHandler.GetFeeInfo().SetGasUsed(gasUnits) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a new field for gasUnitsRefunded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored
Reasoning behind the pull request
gasUsed
andfee
fields forrelayed v3
transactionsProposed changes
mx-chain-core-go
: Extra field in FeeInfo structure mx-chain-core-go#330mx-chain-es-indexer-go
: Inner transactions in operations index mx-chain-es-indexer-go#298Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
feat
branch created?feat
branch merging, do all satellite projects have a proper tag insidego.mod
?