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

[ENG-222] fees improve test coverage #586

Merged
merged 13 commits into from
May 12, 2022

Conversation

loredanacirstea
Copy link
Contributor

@loredanacirstea loredanacirstea commented May 9, 2022

Description

  • fix deployer => registered fees store. Now we use a KVStore for each deployer (deployer address is part of the store prefix). The registered contract address is a key and a non-zero value []byte("1") is used to show existence. This approach makes it easier to add, remove, iterate over all registered contracts of a deployer, compared to the original approach of storing an array of contract addresses for each deployer (all deployers in the same KVStore)
  • add tests for types/params.go, types/codec.go, keeper/fees.go, keeper/grpc_query.go
  • Use ethermint.ValidateNonZeroAddress for validation

Closes: #552


All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

PR review checkboxes:

I have...

  • added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • included the correct type prefix in the PR title
  • targeted the correct branch (see PR Targeting)
  • provided a link in the PR description to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all required CI checks have passed

Code maintenance:

I have...

  • written unit and integration tests
  • added relevant godoc and code comments.
  • updated relevant documentation (docs/) or specification (x/<module>/spec/)

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • confirmed that this PR does not change production code

@linear
Copy link

linear bot commented May 9, 2022

@github-actions github-actions bot added the CLI label May 9, 2022
Each deployer now has a KVStore for storing its registered contracts.
@codecov
Copy link

codecov bot commented May 10, 2022

Codecov Report

Merging #586 (c072cc6) into main (9bf1ae3) will increase coverage by 2.34%.
The diff coverage is 93.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #586      +/-   ##
==========================================
+ Coverage   83.32%   85.67%   +2.34%     
==========================================
  Files         116      117       +1     
  Lines        6376     6372       -4     
==========================================
+ Hits         5313     5459     +146     
+ Misses        935      784     -151     
- Partials      128      129       +1     
Impacted Files Coverage Δ
x/fees/keeper/keeper.go 100.00% <ø> (ø)
x/fees/types/keys.go 0.00% <0.00%> (ø)
x/fees/keeper/grpc_query.go 94.00% <90.47%> (+94.00%) ⬆️
app/app.go 84.69% <100.00%> (ø)
x/fees/keeper/fees.go 98.24% <100.00%> (+43.19%) ⬆️
x/fees/keeper/msg_server.go 94.82% <100.00%> (+0.04%) ⬆️
x/fees/types/fee.go 100.00% <100.00%> (ø)
x/fees/types/msg.go 100.00% <100.00%> (ø)
x/fees/types/params.go 91.17% <0.00%> (+22.05%) ⬆️
... and 2 more

@loredanacirstea loredanacirstea marked this pull request as ready for review May 11, 2022 02:51
app/app.go Show resolved Hide resolved
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.

LGTM minor comments

x/fees/keeper/fees_test.go Outdated Show resolved Hide resolved
x/fees/keeper/fees_test.go Outdated Show resolved Hide resolved
x/fees/keeper/fees_test.go Outdated Show resolved Hide resolved
loredanacirstea and others added 2 commits May 12, 2022 12:09
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Copy link
Contributor

@danburck danburck left a comment

Choose a reason for hiding this comment

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

LGTM, heads up, I added a comment that's not related to the tests.

x/fees/keeper/fees.go Show resolved Hide resolved
@fedekunze fedekunze merged commit a787f95 into main May 12, 2022
@fedekunze fedekunze deleted the loredana/ENG-222-fees-improve-test-coverage branch May 12, 2022 15:34
loredanacirstea added a commit that referenced this pull request May 14, 2022
* main:
  Correct inflation.proto typo (#608)
  change algolia search key (#607)
  Changing erroneous epoch skips to daily instead of weekly (#554)
  fix: remove dup set claims record + CLI update (#605)
  build(deps): bump github.com/spf13/cast from 1.4.1 to 1.5.0 (#604)
  imp: fees test coverage (#586)
  fees: limit address derivation to 20 iterations at registration (#603)
loredanacirstea added a commit to loredanacirstea/evmos that referenced this pull request May 16, 2022
* main:
  Correct inflation.proto typo (evmos#608)
  change algolia search key (evmos#607)
  Changing erroneous epoch skips to daily instead of weekly (evmos#554)
  fix: remove dup set claims record + CLI update (evmos#605)
  build(deps): bump github.com/spf13/cast from 1.4.1 to 1.5.0 (evmos#604)
  imp: fees test coverage (evmos#586)
  fees: limit address derivation to 20 iterations at registration (evmos#603)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve test coverage in x/fees
4 participants