Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

store base fee in event #673

Merged
merged 3 commits into from
Oct 15, 2021
Merged

Conversation

thomas-nguy
Copy link
Contributor

Closes: #XXX

Description


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)

@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #673 (0331319) into main (75d5536) will decrease coverage by 0.12%.
The diff coverage is 0.00%.

❗ Current head 0331319 differs from pull request most recent head bb4b925. Consider uploading reports for the commit bb4b925 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #673      +/-   ##
==========================================
- Coverage   57.02%   56.89%   -0.13%     
==========================================
  Files          63       63              
  Lines        5547     5559      +12     
==========================================
  Hits         3163     3163              
- Misses       2221     2233      +12     
  Partials      163      163              
Impacted Files Coverage Δ
rpc/ethereum/types/utils.go 0.00% <0.00%> (ø)

resParams, err := e.queryClient.FeeMarket.Params(types.ContextWithHeight(height), &feemarkettypes.QueryParamsRequest{})
if err != nil {
return nil, err
}

baseFee := big.NewInt(resParams.Params.InitialBaseFee)
baseFee = big.NewInt(resParams.Params.InitialBaseFee)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be Genesis baseFee, not InitialBaseFee there, but logic is not completely clear

@thomas-nguy thomas-nguy force-pushed the thomas/base-fee-event branch 7 times, most recently from b108175 to a8ab919 Compare October 14, 2021 08:20
@thomas-nguy thomas-nguy changed the title [draft]store base fee in event store base fee in event Oct 14, 2021
@thomas-nguy thomas-nguy force-pushed the thomas/base-fee-event branch from a8ab919 to 1fdb0b0 Compare October 14, 2021 08:47
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Ack minor comment. Can you add a changelog entry?

@@ -80,6 +80,10 @@ func (p Params) Validate() error {
return nil
}

func (p *Params) IsBaseFeeEnable(height int64) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func (p *Params) IsBaseFeeEnable(height int64) bool {
func (p *Params) IsBaseFeeEnabled(height int64) bool {

rpc/ethereum/backend/backend.go Show resolved Hide resolved
@thomas-nguy thomas-nguy force-pushed the thomas/base-fee-event branch from e70e305 to a793e61 Compare October 15, 2021 01:06
@thomas-nguy thomas-nguy force-pushed the thomas/base-fee-event branch from b29fca7 to 0331319 Compare October 15, 2021 01:19
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

ACK

@fedekunze fedekunze enabled auto-merge (squash) October 15, 2021 08:55
@fedekunze fedekunze merged commit cd3b0be into evmos:main Oct 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants