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
added fee in elastic search transactions #2658
Conversation
process/economics/economicsData.go
Outdated
|
||
func (ed *economicsData) computeGasRefund(refundValue *big.Int, gasPrice uint64) uint64 { | ||
gasPriceBig := big.NewInt(0).SetUint64(gasPrice) | ||
|
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.
delete empty line
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.
process/economics/economicsData.go
Outdated
gasPriceBig := big.NewInt(0).SetUint64(gasPrice) | ||
|
||
gasRefund := big.NewInt(0).Div(refundValue, gasPriceBig).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.
delete empty line
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.
…or-cases small-improvement-of-return-message
core/indexer/common.go
Outdated
return true | ||
} | ||
} | ||
|
||
return false | ||
} | ||
|
||
func isRelayedTx(tx *Transaction) bool { | ||
if strings.HasPrefix(string(tx.Data), "relayedTx") && len(tx.SmartContractResults) > 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.
You can use one line here: return strings.HasPrefix(string(tx.Data), "relayedTx") && len(tx.SmartContractResults) > 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.
done.
@@ -159,8 +159,8 @@ func checkDataIndexerParams(arguments *ArgsIndexerFactory) error { | |||
if check.IfNil(arguments.EpochStartNotifier) { | |||
return core.ErrNilEpochStartNotifier | |||
} | |||
if arguments.FeeConfig == nil { | |||
return core.ErrNilFeeConfig | |||
if arguments.TransactionFeeCalculator == nil { |
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.
check.IfNil
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.
core/indexer/processTransactions.go
Outdated
if isRelayedTx(tx) { | ||
tx.GasUsed = tx.GasLimit | ||
fee := tdp.txFeeCalculator.ComputeTxFeeBasedOnGasUsed(tx, tx.GasUsed) | ||
transactions[hash].Fee = fee.String() |
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.
tx instead transactions[hash]
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.
core/indexer/processTransactions.go
Outdated
continue | ||
} | ||
|
||
transactions[hash].Status = transaction.TxStatusFail.String() | ||
tx.Status = transaction.TxStatusFail.String() |
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 think you can remove lines 113-118 and just add this line inside -> if !isRelayedTx(tx), and this way you remove the duplicated code 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.
done.
process/economics/economicsData.go
Outdated
} | ||
|
||
gasRefund := ed.computeGasRefund(refundValue, tx.GetGasPrice()) | ||
gasUsed := tx.GetGasLimit() - gasRefund |
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.
Use safeSub with log
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.
|
||
gasRefund := big.NewInt(0).Div(refundValue, gasPriceBig).Uint64() | ||
|
||
gasRefund = uint64(ed.GasPriceModifier() * float64(gasRefund)) |
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.
So we will assume that any refunded value would have been used with gas price modifier applied @sasurobert
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.
ed.GasPriceModifier return 1.0 when it is disabled
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.
Yes I know, but I have referred here to all the situations after the flag activation.
|
||
gasRefund := big.NewInt(0).Div(refundValue, gasPriceBig).Uint64() | ||
|
||
gasRefund = uint64(ed.GasPriceModifier() * float64(gasRefund)) |
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.
Yes I know, but I have referred here to all the situations after the flag activation.
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.
System tests passed
Added fee in transaction that is indexed in elasticsearch database.
Added
gasPriceModifer
in the api route/network/config