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

shield: implement distribution params #380

Merged
merged 18 commits into from
Jun 8, 2022
Merged

Conversation

yoongbok-lee
Copy link
Collaborator

Closes: #379
Related: #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]"

@yoongbok-lee yoongbok-lee marked this pull request as draft May 25, 2022 05:48
@yoongbok-lee yoongbok-lee changed the title shield edits without proto gen shield: implement distribution params May 25, 2022
@yoongbok-lee yoongbok-lee marked this pull request as ready for review May 26, 2022 09:19
Copy link
Contributor

@xiumingdou xiumingdou left a comment

Choose a reason for hiding this comment

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

Logically LGTM.
Seems test case TestBeginBlocker panicked.
=== RUN TestBeginBlocker/normal --- FAIL: TestBeginBlocker (0.04s) --- FAIL: TestBeginBlocker/normal (0.00s) panic: invalid coin denominations; stake, uctk [recovered] panic: invalid coin denominations; stake, uctk

Copy link
Contributor

@xiumingdou xiumingdou left a comment

Choose a reason for hiding this comment

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

Just curious, how do we test upgrade?

Copy link
Contributor

@eRuaro eRuaro left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #380 (308ca1a) into master (1ff5002) will increase coverage by 1.36%.
The diff coverage is 26.31%.

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
+ Coverage   47.46%   48.83%   +1.36%     
==========================================
  Files         102      110       +8     
  Lines        8258     8550     +292     
==========================================
+ Hits         3920     4175     +255     
+ Misses       3927     3921       -6     
- Partials      411      454      +43     
Impacted Files Coverage Δ
app/config.go 0.00% <0.00%> (ø)
x/gov/keeper/proposal.go 42.75% <ø> (+3.44%) ⬆️
x/gov/keeper/vote.go 41.28% <ø> (-11.01%) ⬇️
x/mint/keeper/keeper.go 54.83% <0.00%> (ø)
x/shield/keeper/grpc_query.go 0.00% <0.00%> (ø)
x/shield/keeper/querier.go 0.45% <0.00%> (-0.03%) ⬇️
x/shield/keeper/rewards.go 0.00% <0.00%> (ø)
x/gov/keeper/deposit.go 61.76% <20.00%> (-9.67%) ⬇️
app/upgrade_handler.go 21.05% <33.33%> (+3.66%) ⬆️
x/shield/keeper/migrations.go 40.00% <40.00%> (ø)
... and 25 more

@yoongbok-lee yoongbok-lee merged commit 817c8b1 into master Jun 8, 2022
@yoongbok-lee yoongbok-lee deleted the shd-distr-params branch June 8, 2022 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement new shield collateral provision reward
4 participants