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

fix: LSM cap room bug #639

Merged
merged 3 commits into from
Sep 6, 2023
Merged

fix: LSM cap room bug #639

merged 3 commits into from
Sep 6, 2023

Conversation

kruspy
Copy link
Collaborator

@kruspy kruspy commented Sep 6, 2023

1. Overview

This change fixes a bug that happens when a validator is tagged as delegable after checking the LSM caps (both validator and bond) but the room left until reaching the cap is less than the amount which is going to be staked by the x/liquidstakeibc module the next delegation epoch.

This causes the module to generate delegating messages without taking into account that room, and therefore potentially hitting any of the caps and starting an endless loop of delegations.

In order to fix this, after calculating the delegable flag when receiving an ICQ validator response (and only when that flag is true), the module will also simulate the generated messages it would create during the next delegation epoch. Then, using the amounts on each of the messages, check if there is enough room to delegate to that validator. If affirmative, the validator keeps tagged as delegable, and as non-delegable otherwise.

@kruspy kruspy requested a review from a team as a code owner September 6, 2023 22:24
@github-actions

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

Coverage after merging marc/lsm-cap-fix into main

72.11%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
x/liquidstakeibc
   genesis.go100%100%100%
   module_ibc.go100%100%8.93%..., 88, 89, 90, 91
   module.go100%100%54.32%..., 72, 73, 74, 99
x/liquidstakeibc/keeper
   delegation_strategy.go100%100%100%
   host_chain.go100%100%99.56%106
   grpc_querier.go100%100%100%
   msg_server.go100%100%60.08%..., 94, 95, 96, 97
   validator_unbonding.go100%100%100%
   invariants.go100%100%100%
   keeper.go100%100%99.09%90, 91
   deposit.go100%100%100%
   abci.go100%100%42.74%..., 79, 80, 81, 82
   lsm_deposit.go100%100%100%
   hooks.go100%100%42.23%..., 732, 733, 734, 74
   ibc.go100%100%37.60%..., 91, 92, 93, 94
   ica.go100%100%65.57%..., 71, 72, 73, 74
   icq_queries.go100%100%100%
   migrations.go100%100%50%20, 21, 22
   ica_handlers.go100%100%32.42%..., 94, 95, 96, 97
   unbonding.go100%100%100%
   icq.go100%100%94.26%..., 173, 39, 40, 41
   user_unbonding.go100%100%100%
x/liquidstakeibc/types
   keys.go100%100%0%..., 94, 96, 97, 98
   genesis.go100%100%100%
   msgs.go100%100%95.06%..., 335, 336, 439, 440
   codec.go100%100%100%
   liquidstakeibc.go100%100%100%
   host_chain.go100%100%100%
   params.go100%100%100%

@kruspy kruspy enabled auto-merge (squash) September 6, 2023 23:05
@github-actions
Copy link

github-actions bot commented Sep 6, 2023

Coverage after merging marc/lsm-cap-fix into main

72.11%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
x/liquidstakeibc
   module.go100%100%54.32%..., 72, 73, 74, 99
   module_ibc.go100%100%8.93%..., 88, 89, 90, 91
   genesis.go100%100%100%
x/liquidstakeibc/keeper
   grpc_querier.go100%100%100%
   user_unbonding.go100%100%100%
   invariants.go100%100%100%
   delegation_strategy.go100%100%100%
   keeper.go100%100%99.09%90, 91
   host_chain.go100%100%99.56%106
   ibc.go100%100%37.60%..., 91, 92, 93, 94
   msg_server.go100%100%60.08%..., 94, 95, 96, 97
   ica.go100%100%65.57%..., 71, 72, 73, 74
   hooks.go100%100%42.23%..., 732, 733, 734, 74
   icq.go100%100%94.26%..., 173, 39, 40, 41
   migrations.go100%100%50%20, 21, 22
   validator_unbonding.go100%100%100%
   abci.go100%100%42.74%..., 79, 80, 81, 82
   icq_queries.go100%100%100%
   lsm_deposit.go100%100%100%
   ica_handlers.go100%100%32.42%..., 94, 95, 96, 97
   unbonding.go100%100%100%
   deposit.go100%100%100%
x/liquidstakeibc/types
   host_chain.go100%100%100%
   liquidstakeibc.go100%100%100%
   msgs.go100%100%95.06%..., 335, 336, 439, 440
   keys.go100%100%0%..., 94, 96, 97, 98
   genesis.go100%100%100%
   params.go100%100%100%
   codec.go100%100%100%

@kruspy kruspy disabled auto-merge September 6, 2023 23:08
@kruspy kruspy merged commit bbfa154 into main Sep 6, 2023
9 checks passed
@kruspy kruspy deleted the marc/lsm-cap-fix branch September 6, 2023 23:08
Copy link
Member

@puneet2019 puneet2019 left a comment

Choose a reason for hiding this comment

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

late lgtm :P

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.

3 participants