fix: reset validator signing info missed blocks counter #8053
Conversation
| @@ -62,20 +63,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| * [#7472](https://github.com/osmosis-labs/osmosis/pull/7472) Refactor TWAP keys to only require a single key format. Significantly lowers TWAP-caused writes | |||
| * [#7499](https://github.com/osmosis-labs/osmosis/pull/7499) Slight speed/gas improvements to CL CreatePosition and AddToPosition | |||
| * [#7564](https://github.com/osmosis-labs/osmosis/pull/7564) Move protorev dev account bank sends from every backrun to once per epoch | |||
| * [#7508](https://github.com/osmosis-labs/osmosis/pull/7508) Improve protorev performance by removing iterator and storing base denoms as a single object rather than an array. | |||
There was a problem hiding this comment.
Sorry, drive by change, periods at the end of some but not all changelog entries was bothering me.
|
Warning Rate Limit Exceeded@czarcas7ic has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 31 minutes and 47 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates to the Osmosis project focus on enhancing validator operations and pruning obsolete data. Changes include resetting the missed blocks counter for validators, removing outdated behaviors in lockups, and speeding up various operations. Additionally, the v25 upgrade introduces necessary tests and functions for these adjustments, ensuring robustness and efficiency in the network's performance. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Actionable comments outside the diff hunks (9)
CHANGELOG.md (9)
Line range hint
983-983: Adjust heading level from h4 to h3 for consistency.- #### [v6.4.0] + ### [v6.4.0]
Line range hint
995-995: Adjust heading level from h4 to h3 for consistency.- #### [v6.3.1] + ### [v6.3.1]
Line range hint
1004-1004: Adjust heading level from h4 to h3 for consistency.- #### [v6.3.0] + ### [v6.3.0]
Line range hint
1009-1009: Adjust heading level from h4 to h3 for consistency.- #### [v6.2.0] + ### [v6.2.0]
Line range hint
1061-1061: Adjust heading level from h4 to h3 for consistency.- #### [v6.1.0] + ### [v6.1.0]
Line range hint
322-322: Remove the period at the end of the heading for consistency.- ## [v4.0.0] + ## [v4.0.0]
Line range hint
585-585: Convert the bare URL to a markdown link for better readability.- See: [SDK v0.43.0 Release Notes](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0) For more details + See: [SDK v0.43.0 Release Notes](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0) for more details.
Line range hint
643-643: Convert the bare URL to a markdown link for better readability.- [wasmd-v.022.0-osmo-v7.2](https://github.com/osmosis-labs/wasmd/releases/tag/v0.22.0-osmo-v7.2) Upgrade SDK and IAVL dependencies to use fast storage + [wasmd-v.022.0-osmo-v7.2](https://github.com/osmosis-labs/wasmd/releases/tag/v0.22.0-osmo-v7.2) Upgrade SDK and IAVL dependencies to use fast storage.
Line range hint
1224-1224: Ensure that all links are properly formatted and not empty.- * [Commit db450f0](https://github.com/osmosis-labs/osmosis/commit/db450f0dce8c595211d920f9bca7ed0f3a136e43) - Revert back to passing in the correct staking keeper into the IBC keeper constructor. + * [Commit db450f0](https://github.com/osmosis-labs/osmosis/commit/db450f0dce8c595211d920f9bca7ed0f3a136e43) - Revert back to passing in the correct staking keeper into the IBC keeper constructor.
app/upgrades/v25/upgrades.go
Outdated
| info.MissedBlocksCounter = 0 | ||
| slashingKeeper.SetValidatorSigningInfo(ctx, address, info) |
There was a problem hiding this comment.
This isn't right -- this will have the opposite of problem of setting a Ceil / potentially leading to negative missed blocks.
You have to re-compute the missed blocks by loading all the bit arrays.
There was a problem hiding this comment.
Oh I see, couldn't we just reset the bit array to zero again for simplicity @ValarDragon
There was a problem hiding this comment.
By bit array to zero I mean, just deleting them and start from where we did in the migration
There was a problem hiding this comment.
Yes that would work! (But they would have to be deleted)
There was a problem hiding this comment.
Good catch btw, my bad for overlooking
There was a problem hiding this comment.
I believe I did it correctly here, and added a test that replicates mainnet better: fbcd43c
Closes: #XXX
What is the purpose of the change
In v24, we upgraded to the sdk v0.50 slashing logic. When timing the migration, we realized that a non-negligible period of time was spent copying over the missed blocks, so we decided it was fine to clear these missed blocks. We, however, forgot to also reset the missed block counter. Therefore, a validator's missed block counter floor is stuck at the value it was at the time of upgrade.
Testing and Verifying
Go test added to demonstrate the counter is reset.
Summary by CodeRabbit