Skip to content

Commit

Permalink
fix: reset validator signing info missed blocks counter (#8053)
Browse files Browse the repository at this point in the history
* reset signing info

* add changelog entry

* nit

* correctly reset missed block counter
  • Loading branch information
czarcas7ic authored Apr 19, 2024
1 parent fe5f836 commit 6b0ac5a
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 11 deletions.
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.
* [#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)
}

0 comments on commit 6b0ac5a

Please sign in to comment.