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

Delegation Staking Limits #1331

Merged
merged 12 commits into from
Feb 2, 2023
Merged

Delegation Staking Limits #1331

merged 12 commits into from
Feb 2, 2023

Conversation

iramiller
Copy link
Member

Description

This PR adds hooks that prevent stake concentration for validators.

closes: #1322


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • 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.
  • 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
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Merging #1331 (d5dee04) into main (bfb3b38) will decrease coverage by 0.07%.
The diff coverage is 30.35%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1331      +/-   ##
==========================================
- Coverage   59.00%   58.93%   -0.07%     
==========================================
  Files         196      197       +1     
  Lines       24729    24783      +54     
==========================================
+ Hits        14591    14606      +15     
- Misses       9046     9085      +39     
  Partials     1092     1092              
Impacted Files Coverage Δ
internal/handlers/staking_restrictions_hooks.go 27.77% <27.77%> (ø)
app/app.go 87.51% <100.00%> (ø)

fix int64 size issue in error message
@iramiller
Copy link
Member Author

iramiller commented Feb 1, 2023

To verify:

make clean build localnet-start
./build/provenanced -t --home ./build/node1/ keys list  

Note address: tp173f8e97r2uft42ltsdhjzz7e8qzv0pr4cxlkd9

./build/provenanced -t --home ./build/node0/ q staking validators

Note address: tpvaloper173f8e97r2uft42ltsdhjzz7e8qzv0pr480zzcq

./build/provenanced -t --home ./build/node1/ tx staking delegate tpvaloper173f8e97r2uft42ltsdhjzz7e8qzv0pr480zzcq 17999999965710000000nhash --from tp173f8e97r2uft42ltsdhjzz7e8qzv0pr4cxlkd9 --chain-id chain-local --fees 38100000000nhash

Note the failure message: 'failed to execute message; message index: 0: validator bonded tokens of 18000000365710000000 exceeds max of 5940000120684300000 (33.0%) for 4 validators: invalid request'

./build/provenanced -t --home ./build/node1/ tx staking delegate tpvaloper173f8e97r2uft42ltsdhjzz7e8qzv0pr480zzcq 10000000nhash --from tp173f8e97r2uft42ltsdhjzz7e8qzv0pr4cxlkd9 --chain-id chain-local --fees 38100000000nhash

Note that this smaller delegation is successful.

@iramiller iramiller marked this pull request as ready for review February 1, 2023 21:25
@iramiller iramiller requested a review from a team as a code owner February 1, 2023 21:25
SpicyLemon
SpicyLemon previously approved these changes Feb 1, 2023
Copy link
Contributor

@SpicyLemon SpicyLemon left a comment

Choose a reason for hiding this comment

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

Needs changelog entry, but looks good.

nullpointer0x00
nullpointer0x00 previously approved these changes Feb 1, 2023
SpicyLemon
SpicyLemon previously approved these changes Feb 1, 2023
@iramiller iramiller enabled auto-merge (squash) February 1, 2023 23:40
@egaxhaj
Copy link
Contributor

egaxhaj commented Feb 2, 2023

codecov has dropped a bit

nullpointer0x00
nullpointer0x00 previously approved these changes Feb 2, 2023
@iramiller
Copy link
Member Author

codecov has dropped a bit

This is expected... the hooks are not reasonable to unit test due to lack of mock structure (especially given the timeframe on release) which is why there is a manual test case on this issue. Not ideal certainly.

@iramiller iramiller merged commit bb9dcfe into main Feb 2, 2023
@iramiller iramiller deleted the 1322-delegation-stake-limits branch May 16, 2023 17:00
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 configurable validator staking limit
4 participants