Problem: missing json rpc of eth_feeHistory #685 #734

leejw51crypto · 2021-11-09T14:25:58Z

Solution: add feehistory api

leejw51crypto · 2021-11-12T05:32:30Z

will add unit test
and will limit count up to 100

because it's heavy task, can be used maliciously

fedekunze · 2021-11-12T08:20:13Z

@leejw51crypto let me know if you need support with this PR, I'd like to add it for the next release

leejw51crypto · 2021-11-12T08:40:59Z

ok

leejw51crypto · 2021-11-15T23:41:39Z

add unit test
rebased

leejw51crypto · 2021-11-16T06:50:33Z

fix golangci-lint

fedekunze

thanks @leejw51crypto, please add more comments and clean up the logic based on the comments. Also if you could add a changelog entry

rpc/ethereum/backend/feebackend.go

fedekunze · 2021-11-16T09:54:06Z

rpc/ethereum/backend/feebackend.go

+	gasUsedString := gasUsedBig.ToInt().String()
+	gasusedfloat, _ := strconv.ParseFloat(gasUsedString, 64)


can you convert to float64 directly without parsing the string?

fedekunze · 2021-11-16T09:54:21Z

rpc/ethereum/backend/feebackend.go

+	if gasLimitUint64 > 0 {
+		gasUsedRatio = gasusedfloat / float64(gasLimitUint64)
+	}


this needs a comment

shouldn't we error if gasLimitUint64 is 0?

yes, i'll return error if gaslimit is 0

fedekunze · 2021-11-16T09:55:49Z

rpc/ethereum/backend/feebackend.go

+type OneFeeHistory struct {
+	BaseFee *big.Int
+	Reward  []*big.Int
+	GasUsed float64
+}


move to types? also add a comment to describe what it is and what it does

fedekunze · 2021-11-16T09:56:08Z

rpc/ethereum/backend/feebackend.go

+	GasUsed float64
+}
+
+func (e *EVMBackend) FeeHistory(userBlockCount rpc.DecimalOrHex, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*rpctypes.FeeHistoryResult, error) {


add comment and split arguments

fedekunze · 2021-11-16T09:58:46Z

rpc/ethereum/backend/feebackend.go

+		if ethBlock == nil || err != nil {
+			return nil, err
+		}


technically this can return nil, nil if both ethBlock and err are nil. is this expected?

with err==nil, ethBlock is also nil

rpc/ethereum/backend/feebackend.go

leejw51crypto · 2021-11-17T01:10:28Z

done

add oracle backend space ready structure ok refactoring return feehistory data flow ok basefee set gas used ratio computing reward add testing add gas used prepare data fill reward increase coin fixing api add mac add launch gas used ratio ok print element reward workes reward working fix panic value correct remove debugging log tidy up tidy up remove oracle tidy up fix handler crash add unit test tidy up add limit check reformat fix lint fix lint fix lint fix lint Update rpc/ethereum/backend/feebackend.go thanks Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Update rpc/ethereum/backend/feebackend.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Update rpc/ethereum/backend/feebackend.go thanks Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Update rpc/ethereum/backend/feebackend.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> fix compile error split lines remove temporary string conversion return error if gaslimit is 0 move OneFeeHistory to types add comment only err check Update rpc/ethereum/backend/feebackend.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Update rpc/ethereum/backend/feebackend.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> tidy up add feehistory-cap

codecov · 2021-11-17T08:51:04Z

Codecov Report

Merging #734 (6c260a6) into main (b7e8dd8) will decrease coverage by 0.03%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##             main     #734      +/-   ##
==========================================
- Coverage   55.86%   55.83%   -0.04%     
==========================================
  Files          64       64              
  Lines        5859     5864       +5     
==========================================
+ Hits         3273     3274       +1     
- Misses       2386     2390       +4     
  Partials      200      200

Impacted Files	Coverage Δ
server/config/config.go	`21.42% <42.85%> (-0.06%)`	⬇️

leejw51crypto · 2021-11-17T08:51:33Z

default history cap is 100 to prevent ddos attack,
can configured with app.toml

# FeeHistoryCap sets the global cap for total number of blocks that can be fetched
feehistory-cap = 100

fedekunze

LGTM minor comments

rpc/ethereum/backend/backend.go

rpc/ethereum/backend/feebackend.go

fedekunze · 2021-11-17T11:40:00Z

rpc/ethereum/backend/feebackend.go

+			tx := ethMsg.AsTransaction()
+			reward := tx.EffectiveGasTipValue(blockBaseFee)
+			sorter[i] = txGasAndReward{gasUsed: txGasUsed, reward: reward}
+			break


I don't understand the purpose of this break here, are we only supporting 1 MsgEthereumTx message?

rpc/ethereum/backend/feebackend.go

rpc/ethereum/types/types.go

leejw51crypto requested review from fedekunze, jolube and khoslaventures as code owners November 9, 2021 14:25

leejw51crypto marked this pull request as draft November 9, 2021 14:26

leejw51crypto force-pushed the feature/685 branch 3 times, most recently from 8f3cb1c to 0e48645 Compare November 15, 2021 23:40

leejw51crypto marked this pull request as ready for review November 15, 2021 23:44

leejw51crypto force-pushed the feature/685 branch 3 times, most recently from dc25728 to 720fbaa Compare November 16, 2021 06:31

fedekunze suggested changes Nov 16, 2021

View reviewed changes

leejw51crypto force-pushed the feature/685 branch 2 times, most recently from 3daab0e to d6692e6 Compare November 17, 2021 06:46

leejw51crypto force-pushed the feature/685 branch from f6cb2ba to 02fcb05 Compare November 17, 2021 08:49

fedekunze reviewed Nov 17, 2021

View reviewed changes

Apply suggestions from code review

c36ea5a

fedekunze approved these changes Nov 17, 2021

View reviewed changes

changelog

8568f36

fedekunze enabled auto-merge (squash) November 17, 2021 11:49

linter fixes

6c260a6

fedekunze merged commit 392d1dd into evmos:main Nov 17, 2021

loredanacirstea mentioned this pull request Mar 14, 2022

eth_feeHistory - fix reward calculation from MsgEthereumTx #990

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem: missing json rpc of eth_feeHistory #685 #734

Problem: missing json rpc of eth_feeHistory #685 #734

leejw51crypto commented Nov 9, 2021

leejw51crypto commented Nov 12, 2021 •

edited

Loading

fedekunze commented Nov 12, 2021

leejw51crypto commented Nov 12, 2021

leejw51crypto commented Nov 15, 2021

leejw51crypto commented Nov 16, 2021

fedekunze left a comment

fedekunze Nov 16, 2021

fedekunze Nov 16, 2021

fedekunze Nov 16, 2021

leejw51crypto Nov 17, 2021

fedekunze Nov 16, 2021

fedekunze Nov 16, 2021

fedekunze Nov 16, 2021

leejw51crypto Nov 17, 2021 •

edited

Loading

leejw51crypto commented Nov 17, 2021 •

edited

Loading

codecov bot commented Nov 17, 2021 •

edited

Loading

leejw51crypto commented Nov 17, 2021

fedekunze left a comment

fedekunze Nov 17, 2021

		gasUsedString := gasUsedBig.ToInt().String()
		gasusedfloat, _ := strconv.ParseFloat(gasUsedString, 64)

Problem: missing json rpc of eth_feeHistory #685 #734

Problem: missing json rpc of eth_feeHistory #685 #734

Conversation

leejw51crypto commented Nov 9, 2021

leejw51crypto commented Nov 12, 2021 • edited Loading

fedekunze commented Nov 12, 2021

leejw51crypto commented Nov 12, 2021

leejw51crypto commented Nov 15, 2021

leejw51crypto commented Nov 16, 2021

fedekunze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leejw51crypto Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

leejw51crypto commented Nov 17, 2021 • edited Loading

codecov bot commented Nov 17, 2021 • edited Loading

Codecov Report

leejw51crypto commented Nov 17, 2021

fedekunze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leejw51crypto commented Nov 12, 2021 •

edited

Loading

leejw51crypto Nov 17, 2021 •

edited

Loading

leejw51crypto commented Nov 17, 2021 •

edited

Loading

codecov bot commented Nov 17, 2021 •

edited

Loading