-
Notifications
You must be signed in to change notification settings - Fork 561
eth_feeHistory - fix reward calculation from MsgEthereumTx #990
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
@@ -61,14 +61,15 @@ func (e *EVMBackend) processBlock( | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -90,29 +91,28 @@ func (e *EVMBackend) processBlock( | |||
if reward == nil { | |||
reward = big.NewInt(0) | |||
} | |||
sorter[i] = txGasAndReward{gasUsed: txGasUsed, reward: reward} | |||
break |
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.
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
MsgEthereumTx
message?
#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
MsgEthereumTx
in a tendermintTx
? (tx.GetMsgs()
) - if yes, on what rules are the
MsgEthereumTx
batched in aTx
?
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, we can find MsgEthereumTx in a Tendermint Tx. There are no filters, we can only iterate over the msgs
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.
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:
MsgEthereumTx
count.sorter := make(sortGasAndReward, tendermintTxCount)
creates an array of{0 <nil>}
values.The
nil
value from thereward
attribute was never rewritten for tendermint Tx without anyMsgEthereumTx
Fixes:
MsgEthereumTx
, as in:https://github.com/ethereum/go-ethereum/blob/57cec892536270fc6dafae01ded2c528ffa370e9/eth/gasprice/feehistory.go#L106-L113
MsgEthereumTx
count to calculate rewardsFixes #974
Closes: #XXX
Description
For contributor use:
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerFor admin use:
WIP
,R4R
,docs
, etc)