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: reset validator signing info missed blocks counter #8053

Merged
merged 5 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### State Breaking

* [#8053](https://github.com/osmosis-labs/osmosis/pull/8053) Reset validator signing info missed blocks counter
* [#8030](https://github.com/osmosis-labs/osmosis/pull/8030) Delete legacy behavior where lockups could not unbond at very small block heights on a testnet.
* [#7005](https://github.com/osmosis-labs/osmosis/pull/7005) Adding deactivated smart account module.

### State Compatible

* [#8006](https://github.com/osmosis-labs/osmosis/pull/8006), [#8014](https://github.com/osmosis-labs/osmosis/pull/8014) Speedup many BigDec operations
* [#8030](https://github.com/osmosis-labs/osmosis/pull/8030) Delete legacy behavior where lockups could not unbond at very small block heights on a testnet

## v24.0.1

Expand All @@ -63,20 +65,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.
Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, drive by change, periods at the end of some but not all changelog entries was bothering me.

* [#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
* [#7509](https://github.com/osmosis-labs/osmosis/pull/7509) Distributing ProtoRev profits to the community pool and burn address
* [#7524](https://github.com/osmosis-labs/osmosis/pull/7524) Poolmanager: ListPoolsByDenom will now skip pools that cannot correctly return their constituent denoms.
* [#7550](https://github.com/osmosis-labs/osmosis/pull/7550) Speedup small CL swaps, by only fetching CL uptime accumulators if there is a tick crossing.
* [#7524](https://github.com/osmosis-labs/osmosis/pull/7524) Poolmanager: ListPoolsByDenom will now skip pools that cannot correctly return their constituent denoms
* [#7550](https://github.com/osmosis-labs/osmosis/pull/7550) Speedup small CL swaps, by only fetching CL uptime accumulators if there is a tick crossing
* [#7555](https://github.com/osmosis-labs/osmosis/pull/7555) Refactor taker fees, distribute via a single module account, track once at epoch
* [#7562](https://github.com/osmosis-labs/osmosis/pull/7562) Speedup Protorev estimation logic by removing unnecessary taker fee simulations.
* [#7595](https://github.com/osmosis-labs/osmosis/pull/7595) Fix cosmwasm pool model code ID migration.
* [#7615](https://github.com/osmosis-labs/osmosis/pull/7615) Min value param for epoch distribution.
* [#7562](https://github.com/osmosis-labs/osmosis/pull/7562) Speedup Protorev estimation logic by removing unnecessary taker fee simulations
* [#7595](https://github.com/osmosis-labs/osmosis/pull/7595) Fix cosmwasm pool model code ID migration
* [#7615](https://github.com/osmosis-labs/osmosis/pull/7615) Min value param for epoch distribution
* [#7619](https://github.com/osmosis-labs/osmosis/pull/7619) Slight speedup/gas improvement to CL GetTotalPoolLiquidity queries
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Create/remove tick events.
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Create/remove tick events
* [#7623](https://github.com/osmosis-labs/osmosis/pull/7623) Add query for querying all before send hooks
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Remove duplicate CL accumulator update logic.
* [#7665](https://github.com/osmosis-labs/osmosis/pull/7665) feat(x/protorev): Use Transient store to store swap backruns.
* [#7685](https://github.com/osmosis-labs/osmosis/pull/7685) Speedup CL actions by only marshalling for CL hooks if they will be used.
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Remove duplicate CL accumulator update logic
* [#7665](https://github.com/osmosis-labs/osmosis/pull/7665) feat(x/protorev): Use Transient store to store swap backruns
* [#7685](https://github.com/osmosis-labs/osmosis/pull/7685) Speedup CL actions by only marshalling for CL hooks if they will be used
* [#7503](https://github.com/osmosis-labs/osmosis/pull/7503) Add IBC wasm light clients module
* [#7689](https://github.com/osmosis-labs/osmosis/pull/7689) Make CL price estimations not cause state writes (speed and gas improvements)
* [#7745](https://github.com/osmosis-labs/osmosis/pull/7745) Add gauge id query to stargate whitelist
Expand All @@ -91,7 +93,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#7857](https://github.com/osmosis-labs/osmosis/pull/7857) SuperfluidDelegationsByValidatorDenom query now returns equivalent staked amount
* [#7912](https://github.com/osmosis-labs/osmosis/pull/7912) Default timeoutCommit to 2s
* [#7951](https://github.com/osmosis-labs/osmosis/pull/7951) Only migrate selected cl incentives
* [#7938](https://github.com/osmosis-labs/osmosis/pull/7938) Add missing swap events for missing swap event for cw pools.
* [#7938](https://github.com/osmosis-labs/osmosis/pull/7938) Add missing swap events for missing swap event for cw pools
* [#7957](https://github.com/osmosis-labs/osmosis/pull/7957) Update to the latest version of ibc-go
* [#7966](https://github.com/osmosis-labs/osmosis/pull/7966) Update all governance migrated white whale pools to code id 641

Expand Down
25 changes: 25 additions & 0 deletions app/upgrades/v25/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"github.com/cosmos/cosmos-sdk/types/module"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

slashing "github.com/cosmos/cosmos-sdk/x/slashing/keeper"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"

"github.com/osmosis-labs/osmosis/v24/app/keepers"
"github.com/osmosis-labs/osmosis/v24/app/upgrades"
)
Expand All @@ -23,8 +26,12 @@ func CreateUpgradeHandler(
return nil, err
}

// Now that all deprecated historical TWAPs have been pruned via v24, we can delete is isPruning state entry as well
keepers.TwapKeeper.DeleteDeprecatedHistoricalTWAPsIsPruning(ctx)

// Reset missed blocks counter for all validators
resetMissedBlocksCounter(ctx, keepers.SlashingKeeper)

// Set the authenticator params in the store
authenticatorParams := keepers.SmartAccountKeeper.GetParams(ctx)
authenticatorParams.MaximumUnauthenticatedGas = 120_000
Expand All @@ -34,3 +41,21 @@ func CreateUpgradeHandler(
return migrations, nil
}
}

// resetMissedBlocksCounter resets the missed blocks counter for all validators back to zero.
// This corrects a mistake that was overlooked in v24, where we cleared all missedBlocks but did not reset the counter.
func resetMissedBlocksCounter(ctx sdk.Context, slashingKeeper *slashing.Keeper) {
// Iterate over all validators signing info
slashingKeeper.IterateValidatorSigningInfos(ctx, func(address sdk.ConsAddress, info slashingtypes.ValidatorSigningInfo) (stop bool) {
missedBlocks, err := slashingKeeper.GetValidatorMissedBlocks(ctx, address)
if err != nil {
panic(err)
}

// Reset missed blocks counter
info.MissedBlocksCounter = int64(len(missedBlocks))
slashingKeeper.SetValidatorSigningInfo(ctx, address, info)

return false
})
}
109 changes: 109 additions & 0 deletions app/upgrades/v25/upgrades_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package v25_test

import (
"testing"
"time"

"github.com/stretchr/testify/suite"

moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
"github.com/cosmos/cosmos-sdk/x/slashing"
v4 "github.com/cosmos/cosmos-sdk/x/slashing/migrations/v4"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

abci "github.com/cometbft/cometbft/abci/types"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/osmosis-labs/osmosis/v24/app/apptesting"
)

const (
v25UpgradeHeight = int64(10)
)

var (
consAddr = sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
)

type UpgradeTestSuite struct {
apptesting.KeeperTestHelper
}

func TestUpgradeTestSuite(t *testing.T) {
suite.Run(t, new(UpgradeTestSuite))
}

func (s *UpgradeTestSuite) TestUpgrade() {
s.Setup()

preMigrationSigningInfo := s.prepareMissedBlocksCounterTest()

// Run the upgrade
dummyUpgrade(s)
s.Require().NotPanics(func() {
s.App.BeginBlocker(s.Ctx, abci.RequestBeginBlock{})
})

s.executeMissedBlocksCounterTest(preMigrationSigningInfo)
}

func dummyUpgrade(s *UpgradeTestSuite) {
s.Ctx = s.Ctx.WithBlockHeight(v25UpgradeHeight - 1)
plan := upgradetypes.Plan{Name: "v25", Height: v25UpgradeHeight}
err := s.App.UpgradeKeeper.ScheduleUpgrade(s.Ctx, plan)
s.Require().NoError(err)
_, exists := s.App.UpgradeKeeper.GetUpgradePlan(s.Ctx)
s.Require().True(exists)

s.Ctx = s.Ctx.WithBlockHeight(v25UpgradeHeight)
}

func (s *UpgradeTestSuite) prepareMissedBlocksCounterTest() slashingtypes.ValidatorSigningInfo {
cdc := moduletestutil.MakeTestEncodingConfig(slashing.AppModuleBasic{}).Codec
slashingStoreKey := s.App.AppKeepers.GetKey(slashingtypes.StoreKey)
store := s.Ctx.KVStore(slashingStoreKey)

// Replicate current mainnet state where, someones missed block counter is greater than their actual missed blocks
preMigrationSigningInfo := slashingtypes.ValidatorSigningInfo{
Address: consAddr.String(),
StartHeight: 10,
IndexOffset: 100,
JailedUntil: time.Time{},
Tombstoned: false,
MissedBlocksCounter: 1000,
}

// Set the missed blocks for the validator
for i := 0; i < 10; i++ {
err := s.App.SlashingKeeper.SetMissedBlockBitmapValue(s.Ctx, consAddr, int64(i), true)
s.Require().NoError(err)
}

// Validate that the missed block bitmap value is of length 10 (the real missed blocks value), differing from the missed blocks counter of 1000 (the incorrect value)
missedBlocks, err := s.App.SlashingKeeper.GetValidatorMissedBlocks(s.Ctx, consAddr)
s.Require().NoError(err)
s.Require().Len(missedBlocks, 10)

// store old signing info and bitmap entries
bz := cdc.MustMarshal(&preMigrationSigningInfo)
store.Set(v4.ValidatorSigningInfoKey(consAddr), bz)

return preMigrationSigningInfo
}

func (s *UpgradeTestSuite) executeMissedBlocksCounterTest(preMigrationSigningInfo slashingtypes.ValidatorSigningInfo) {
postMigrationSigningInfo, found := s.App.SlashingKeeper.GetValidatorSigningInfo(s.Ctx, consAddr)
s.Require().True(found)

// Check that the missed blocks counter was set to the correct value
s.Require().Equal(int64(10), postMigrationSigningInfo.MissedBlocksCounter)

// Check that all other fields are the same
s.Require().Equal(preMigrationSigningInfo.Address, postMigrationSigningInfo.Address)
s.Require().Equal(preMigrationSigningInfo.StartHeight, postMigrationSigningInfo.StartHeight)
s.Require().Equal(preMigrationSigningInfo.IndexOffset, postMigrationSigningInfo.IndexOffset)
s.Require().Equal(preMigrationSigningInfo.JailedUntil, postMigrationSigningInfo.JailedUntil)
s.Require().Equal(preMigrationSigningInfo.Tombstoned, postMigrationSigningInfo.Tombstoned)
}