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

Fix duplicate registration of feemarket GenesisState and Params #1023

Merged
merged 3 commits into from
Apr 5, 2022

Conversation

nddeluca
Copy link
Contributor

@nddeluca nddeluca commented Mar 30, 2022

Description

v0.12.2 introduced legacy types which attempt to register on init, conflicting with the existing types.

 > ethermintd version
2022/03/30 23:10:02 proto: duplicate proto type registered: ethermint.feemarket.v1.Params
2022/03/30 23:10:02 proto: duplicate proto type registered: ethermint.feemarket.v1.GenesisState
0.12.2

This also results in the grpc server showing an incorrect description of the Params type.

 > grpcurl -plaintext localhost:9090 describe .ethermint.feemarket.v1.Params
ethermint.feemarket.v1.Params is a message:
message Params {
  bool no_base_fee = 1;
  uint32 base_fee_change_denominator = 2;
  uint32 elasticity_multiplier = 3;
  int64 initial_base_fee = 4;
  int64 enable_height = 5;
}

The proto registration isn't needed for migrations, so we should be fine in just removing the init calls in the legacy types.

This PR fixes the warnings on init, and results in the correct registration for the feemarket Params.

> grpcurl -plaintext localhost:9090 describe .ethermint.feemarket.v1.Params
ethermint.feemarket.v1.Params is a message:
message Params {
  bool no_base_fee = 1;
  uint32 base_fee_change_denominator = 2;
  uint32 elasticity_multiplier = 3;
  int64 enable_height = 5;
  string base_fee = 6 [(.gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int", (.gogoproto.nullable) = false];
  reserved 4;
  reserved "initial_base_fee";
}

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)

@nddeluca nddeluca changed the title Fix duplication registration of feemarket GenesisState and Params Fix duplicate registration of feemarket GenesisState and Params Mar 30, 2022
@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #1023 (a01ac42) into main (7d11c93) will increase coverage by 0.16%.
The diff coverage is n/a.

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

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1023      +/-   ##
==========================================
+ Coverage   59.16%   59.32%   +0.16%     
==========================================
  Files          80       80              
  Lines        6577     6577              
==========================================
+ Hits         3891     3902      +11     
+ Misses       2467     2456      -11     
  Partials      219      219              
Impacted Files Coverage Δ
x/feemarket/migrations/v010/migrate.go 85.41% <0.00%> (+22.91%) ⬆️

@fedekunze fedekunze enabled auto-merge (squash) April 5, 2022 15:39
@fedekunze fedekunze merged commit ea3c16e into evmos:main Apr 5, 2022
@fedekunze fedekunze mentioned this pull request May 31, 2022
11 tasks
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.

None yet

2 participants