Skip to content
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

[blockchain] Exclude zero values from gas price metrics #337

Merged
merged 2 commits into from
Apr 3, 2023

Conversation

DarianShawn
Copy link
Collaborator

Description

The maximum and average gas price metrics were disturbed by the zero values of the system contract transaction. With zero values adopted, they do not accurately reflect actual user gas prices.

The PR fixes this, by removing miner transactions from transaction count, and providing price bottom limit to metric collector when no user transactions are included.

Changes include

  • Bugfix (non-breaking change that solves an issue)

Testing

  • I have tested this code with the official test suite

@DarianShawn DarianShawn added the bug fix Functionality that fixes a bug label Mar 31, 2023
@DarianShawn DarianShawn self-assigned this Mar 31, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #337 (28202d7) into dev (8b45344) will increase coverage by 0.11%.
The diff coverage is 31.81%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##              dev     #337      +/-   ##
==========================================
+ Coverage   47.37%   47.48%   +0.11%     
==========================================
  Files         135      135              
  Lines       20440    20451      +11     
==========================================
+ Hits         9683     9711      +28     
+ Misses       9919     9908      -11     
+ Partials      838      832       -6     
Impacted Files Coverage Δ
blockchain/metrics.go 11.49% <0.00%> (ø)
blockchain/blockchain.go 46.21% <30.00%> (-0.40%) ⬇️
blockchain/testing.go 60.46% <100.00%> (+0.18%) ⬆️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link

@0xcb9ff9 0xcb9ff9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@DarianShawn DarianShawn merged commit 2127e5b into dev Apr 3, 2023
@DarianShawn DarianShawn deleted the fix-price-metrics-should-exclude-zero-value branch April 3, 2023 07:45
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug fix Functionality that fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants