eth_feeHistory - fix reward calculation from MsgEthereumTx #990
Conversation
Problem:
- rewards calculation based on percentiles took into consideration the tendermint tx count instead of `MsgEthereumTx` count.
- `sorter := make(sortGasAndReward, tendermintTxCount)` creates an array of `{0 <nil>}` values.
The `nil` value from the `reward` attribute was never rewritten for tendermint Tx without any `MsgEthereumTx`
Fixes:
- return early if there are 0 `MsgEthereumTx`
- use the `MsgEthereumTx` count to calculate rewards
| rewardCount := len(rewardPercentiles) | ||
| targetOneFeeHistory.Reward = make([]*big.Int, rewardCount) | ||
| for i := 0; i < rewardCount; i++ { | ||
| targetOneFeeHistory.Reward[i] = big.NewInt(2000) |
There was a problem hiding this comment.
2000 was chosen as default in https://github.com/tharsis/ethermint/pull/734/files#diff-4d996264fd51e2936260254650a034814a870ddfa9f7df7398d53feba7c39ef6R64.
Standard default value is 0 and it was usually rewritten anyway by the code below this line.
| reward = big.NewInt(0) | ||
| } | ||
| sorter[i] = txGasAndReward{gasUsed: txGasUsed, reward: reward} | ||
| break |
There was a problem hiding this comment.
Why the break? I see this same question was asked by @fedekunze :
I don't understand the purpose of this break here, are we only supporting 1
MsgEthereumTxmessage?
#734 (comment)
But I did not find the answer to it.
From what I understand, we should calculate the reward only from MsgEthereumTx and not from tendermintTx. But I have some questions:
- can we find multiple
MsgEthereumTxin a tendermintTx? (tx.GetMsgs()) - if yes, on what rules are the
MsgEthereumTxbatched in aTx?
There was a problem hiding this comment.
yes, we can find MsgEthereumTx in a Tendermint Tx. There are no filters, we can only iterate over the msgs
fedekunze
left a comment
There was a problem hiding this comment.
ACK, can you add a CHANGELOG entry? 👍
* eth_feeHistory - fix reward calculation from MsgEthereumTx
Problem:
- rewards calculation based on percentiles took into consideration the tendermint tx count instead of `MsgEthereumTx` count.
- `sorter := make(sortGasAndReward, tendermintTxCount)` creates an array of `{0 <nil>}` values.
The `nil` value from the `reward` attribute was never rewritten for tendermint Tx without any `MsgEthereumTx`
Fixes:
- return early if there are 0 `MsgEthereumTx`
- use the `MsgEthereumTx` count to calculate rewards
* Add change log
Problem:
MsgEthereumTxcount.sorter := make(sortGasAndReward, tendermintTxCount)creates an array of{0 <nil>}values.The
nilvalue from therewardattribute was never rewritten for tendermint Tx without anyMsgEthereumTxFixes:
MsgEthereumTx, as in:https://github.com/ethereum/go-ethereum/blob/57cec892536270fc6dafae01ded2c528ffa370e9/eth/gasprice/feehistory.go#L106-L113
MsgEthereumTxcount to calculate rewardsFixes #974
Closes: #XXX
Description
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)