fix(rpc): align nextBaseFee and invalid check for fee history #1720

mmsqe · 2023-03-22T10:17:48Z

next block fee for historical block should not change based on latest block
handle request beyond head block
invalid reward percentile

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

tests/integration_tests/test_fee_history.py

rpc/backend/chain_info.go

 				}
-			}
+			}(int32(blockID - blockStart + int64(i)))


rpc/backend/chain_info.go

+			go func(index int32) {
+				defer wg.Done()
+				// fetch block
+				// tendermint block
+				blockNum := rpctypes.BlockNumber(blockStart + int64(index))
+				tendermintblock, err := b.TendermintBlockByNumber(blockNum)
+				if tendermintblock == nil {
+					chanErr <- err
+					return
+				}

-	// fetch block
-	for blockID := blockStart; blockID <= blockEnd; blockID++ {
-		index := int32(blockID - blockStart)
-		// tendermint block
-		tendermintblock, err := b.TendermintBlockByNumber(rpctypes.BlockNumber(blockID))
-		if tendermintblock == nil {
-			return nil, err
-		}
-
-		// eth block
-		ethBlock, err := b.GetBlockByNumber(rpctypes.BlockNumber(blockID), true)
-		if ethBlock == nil {
-			return nil, err
-		}
+				// eth block
+				ethBlock, err := b.GetBlockByNumber(blockNum, true)
+				if ethBlock == nil {
+					chanErr <- err
+					return
+				}

-		// tendermint block result
-		tendermintBlockResult, err := b.TendermintBlockResultByNumber(&tendermintblock.Block.Height)
-		if tendermintBlockResult == nil {
-			b.logger.Debug("block result not found", "height", tendermintblock.Block.Height, "error", err.Error())
-			return nil, err
-		}
+				// tendermint block result
+				tendermintBlockResult, err := b.TendermintBlockResultByNumber(&tendermintblock.Block.Height)
+				if tendermintBlockResult == nil {
+					b.logger.Debug("block result not found", "height", tendermintblock.Block.Height, "error", err.Error())
+					chanErr <- err
+					return
+				}

-		oneFeeHistory := rpctypes.OneFeeHistory{}
-		err = b.processBlock(tendermintblock, &ethBlock, rewardPercentiles, tendermintBlockResult, &oneFeeHistory)
-		if err != nil {
-			return nil, err
-		}
+				oneFeeHistory := rpctypes.OneFeeHistory{}
+				err = b.processBlock(tendermintblock, &ethBlock, rewardPercentiles, tendermintBlockResult, &oneFeeHistory)
+				if err != nil {
+					chanErr <- err
+					return
+				}

-		// copy
-		thisBaseFee[index] = (*hexutil.Big)(oneFeeHistory.BaseFee)
-		thisBaseFee[index+1] = (*hexutil.Big)(oneFeeHistory.NextBaseFee)
-		thisGasUsedRatio[index] = oneFeeHistory.GasUsedRatio
-		if calculateRewards {
-			for j := 0; j < rewardCount; j++ {
-				reward[index][j] = (*hexutil.Big)(oneFeeHistory.Reward[j])
-				if reward[index][j] == nil {
-					reward[index][j] = (*hexutil.Big)(big.NewInt(0))
+				// copy
+				thisBaseFee[index] = (*hexutil.Big)(oneFeeHistory.BaseFee)
+				thisBaseFee[index+1] = (*hexutil.Big)(oneFeeHistory.NextBaseFee)
+				thisGasUsedRatio[index] = oneFeeHistory.GasUsedRatio
+				if calculateRewards {
+					for j := 0; j < rewardCount; j++ {
+						reward[index][j] = (*hexutil.Big)(oneFeeHistory.Reward[j])
+						if reward[index][j] == nil {
+							reward[index][j] = (*hexutil.Big)(big.NewInt(0))
+						}
+					}
 				}
-			}
+			}(int32(blockID - blockStart + int64(i)))


rpc/backend/chain_info.go

+		go func() {
+			wg.Wait()
+			close(wgDone)
+		}()


codecov · 2023-03-25T01:08:26Z

Codecov Report

Merging #1720 (9e06e1f) into main (10d9579) will decrease coverage by 0.98%.
The diff coverage is 68.22%.

❗ Current head 9e06e1f differs from pull request most recent head 4348828. Consider uploading reports for the commit 4348828 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1720      +/-   ##
==========================================
- Coverage   68.89%   67.92%   -0.98%     
==========================================
  Files         117      112       -5     
  Lines       10681    10280     -401     
==========================================
- Hits         7359     6983     -376     
+ Misses       2900     2884      -16     
+ Partials      422      413       -9

Impacted Files	Coverage Δ
rpc/backend/utils.go	`45.81% <51.85%> (+2.31%)`	⬆️
rpc/backend/chain_info.go	`84.12% <84.90%> (+1.38%)`	⬆️

... and 13 files with indirect coverage changes

* fix blk number for next base fee * add test * add change doc * cross check next fee * calc base fee based on params elasticity_multiplier & base_fee_change_denominator * concurrent call with maxBlockFetchers * test with get feemarket params * Update CHANGELOG.md --------- Signed-off-by: yihuang <huang@crypto.com> Co-authored-by: yihuang <huang@crypto.com>

elasticity_multiplier & base_fee_change_denominator

rpc/backend/chain_info.go

 		blockEnd = int64(blockNumber)
+	} else if int64(blockNumber) < blockEnd {
+		return nil, fmt.Errorf("%w: requested %d, head %d", errRequestBeyondHead, blockEnd, int64(blockNumber))


rpc/backend/chain_info.go

 		blockEnd = int64(blockNumber)
+	} else if int64(blockNumber) < blockEnd {


… (#241) * align request beyond head block * align invalid reward percentile

* fix blk number for next base fee * add test * add change doc * cross check next fee * calc base fee based on params elasticity_multiplier & base_fee_change_denominator * concurrent call with maxBlockFetchers * test with get feemarket params * Update CHANGELOG.md --------- Signed-off-by: yihuang <huang@crypto.com> Co-authored-by: yihuang <huang@crypto.com>

… (#241) * align request beyond head block * align invalid reward percentile

* fix(rpc): nextBaseFee for fee history (backport: evmos#1720) (#234) * fix blk number for next base fee * add test * add change doc * cross check next fee * calc base fee based on params elasticity_multiplier & base_fee_change_denominator * concurrent call with maxBlockFetchers * test with get feemarket params * Update CHANGELOG.md --------- Signed-off-by: yihuang <huang@crypto.com> Co-authored-by: yihuang <huang@crypto.com> * fix(rpc): add more invalid check for fee history (backport: evmos#1720) (#241) * align request beyond head block * align invalid reward percentile --------- Signed-off-by: yihuang <huang@crypto.com> Co-authored-by: yihuang <huang@crypto.com>

github-actions · 2023-06-25T00:26:28Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

mmsqe marked this pull request as ready for review March 22, 2023 13:12

mmsqe requested a review from a team as a code owner March 22, 2023 13:12

mmsqe requested review from 0a1c and GAtom22 and removed request for a team March 22, 2023 13:12

mmsqe force-pushed the fix_fee_history branch 2 times, most recently from fceb6ac to 7d2b87b Compare March 23, 2023 11:13

mmsqe commented Mar 23, 2023

View reviewed changes

tests/integration_tests/test_fee_history.py Show resolved Hide resolved

github-advanced-security bot found potential problems Mar 23, 2023

View reviewed changes

rpc/backend/chain_info.go

}

}

}(int32(blockID - blockStart + int64(i)))

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion

Potential integer overflow by integer type conversion

github-advanced-security bot found potential problems Mar 23, 2023

View reviewed changes

mmsqe mentioned this pull request Mar 28, 2023

fix(rpc): nextBaseFee for fee history (backport: #1720) crypto-org-chain/ethermint#234

Merged

11 tasks

mmsqe and others added 12 commits May 10, 2023 09:24

update nix

ad5f4e3

fix blk number for next base fee

dbd443f

add test

69261db

add change doc

2678c6c

cross check next fee

885cc71

calc base fee based on params

0996618

elasticity_multiplier & base_fee_change_denominator

concurrent call with maxBlockFetchers

061f46f

test with get feemarket params

326b2d9

fix test

58f3d25

Update CHANGELOG.md

4756f57

align request beyond head block

e584d17

align invalid reward percentile

4348828

mmsqe force-pushed the fix_fee_history branch from 9e06e1f to 4348828 Compare May 10, 2023 01:25

github-advanced-security bot found potential problems May 10, 2023

View reviewed changes

mmsqe changed the title ~~fix(rpc): nextBaseFee for fee history~~ fix(rpc): align nextBaseFee and invalid check for fee history May 10, 2023

mmsqe added a commit to crypto-org-chain/ethermint that referenced this pull request May 15, 2023

fix(rpc): add more invalid check for fee history (backport: evmos#1720)…

64a0800

… (#241) * align request beyond head block * align invalid reward percentile

mmsqe added a commit to mmsqe/ethermint that referenced this pull request Jun 14, 2023

fix(rpc): add more invalid check for fee history (backport: evmos#1720)…

4154a8f

… (#241) * align request beyond head block * align invalid reward percentile

mmsqe mentioned this pull request Jun 14, 2023

fix: fee history api (backport: #1720) crypto-org-chain/ethermint#271

Merged

11 tasks

github-actions bot added the Status: Stale label Jun 25, 2023

github-actions bot closed this Jul 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(rpc): align nextBaseFee and invalid check for fee history #1720

fix(rpc): align nextBaseFee and invalid check for fee history #1720

mmsqe commented Mar 22, 2023 •

edited

Loading

codecov bot commented Mar 25, 2023 •

edited

Loading

github-actions bot commented Jun 25, 2023

		blockEnd = int64(blockNumber)
		} else if int64(blockNumber) < blockEnd {

fix(rpc): align nextBaseFee and invalid check for fee history #1720

fix(rpc): align nextBaseFee and invalid check for fee history #1720

Conversation

mmsqe commented Mar 22, 2023 • edited Loading

codecov bot commented Mar 25, 2023 • edited Loading

Codecov Report

github-actions bot commented Jun 25, 2023

mmsqe commented Mar 22, 2023 •

edited

Loading

codecov bot commented Mar 25, 2023 •

edited

Loading