From b1fe8aa72e13593bcaad5d91d25fc3b9d44a0408 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 15:45:07 -0500 Subject: [PATCH 1/9] `UpdatedValidatorsCacheKey` --- keeper/keeper.go | 12 +++++++----- keys.go | 3 +++ simapp/test_node.sh | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/keeper/keeper.go b/keeper/keeper.go index 4df7495..636bcaa 100644 --- a/keeper/keeper.go +++ b/keeper/keeper.go @@ -30,9 +30,10 @@ type Keeper struct { logger log.Logger // state management - Schema collections.Schema - Params collections.Item[poa.Params] - PendingValidators collections.Item[poa.Validators] + Schema collections.Schema + Params collections.Item[poa.Params] + PendingValidators collections.Item[poa.Validators] + UpdatedValidatorsCache collections.KeySet[string] CachedBlockPower collections.Item[poa.PowerCache] AbsoluteChangedInBlockPower collections.Item[poa.PowerCache] @@ -59,8 +60,9 @@ func NewKeeper( logger: logger, // Stores - Params: collections.NewItem(sb, poa.ParamsKey, "params", codec.CollValue[poa.Params](cdc)), - PendingValidators: collections.NewItem(sb, poa.PendingValidatorsKey, "pending", codec.CollValue[poa.Validators](cdc)), + Params: collections.NewItem(sb, poa.ParamsKey, "params", codec.CollValue[poa.Params](cdc)), + PendingValidators: collections.NewItem(sb, poa.PendingValidatorsKey, "pending", codec.CollValue[poa.Validators](cdc)), + UpdatedValidatorsCache: collections.NewKeySet(sb, poa.UpdatedValidatorsCacheKey, "updated_validators", collections.StringKey), CachedBlockPower: collections.NewItem(sb, poa.CachedPreviousBlockPowerKey, "cached_block", codec.CollValue[poa.PowerCache](cdc)), AbsoluteChangedInBlockPower: collections.NewItem(sb, poa.AbsoluteChangedInBlockPowerKey, "absolute_changed_power", codec.CollValue[poa.PowerCache](cdc)), diff --git a/keys.go b/keys.go index 84a602c..c4f39ae 100644 --- a/keys.go +++ b/keys.go @@ -17,6 +17,9 @@ var ( // AbsoluteChangedInBlockPowerKey tracks the current blocks total power amount. // If this becomes >30% of CachedPreviousBlockPowerKey, messages will fail to limit IBC issues. AbsoluteChangedInBlockPowerKey = collections.NewPrefix(3) + + // UpdatedValidatorsCacheKey tracks recently updated validators from SetPower. + UpdatedValidatorsCacheKey = collections.NewPrefix(4) ) const ( diff --git a/simapp/test_node.sh b/simapp/test_node.sh index 5f70608..54b2512 100644 --- a/simapp/test_node.sh +++ b/simapp/test_node.sh @@ -3,7 +3,7 @@ # Example: : ' cd simapp -BINARY="poad" CHAIN_ID="poa-1" HOME_DIR="$HOME/.poad" TIMEOUT_COMMIT="2500ms" CLEAN=true sh test_node.sh +BINARY="poad" CHAIN_ID="poa-1" HOME_DIR="$HOME/.poad" TIMEOUT_COMMIT="1200ms" CLEAN=true sh test_node.sh FLAGS="--keyring-backend=test --chain-id=poa-1 --home="$HOME/.poad" --yes" From ac395b0457ae8eee167dc8f7081cc9b2e79773c3 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 15:45:37 -0500 Subject: [PATCH 2/9] ABCI: `DeleteValidatorByPowerIndex` logic --- keeper/poa.go | 39 ++++++++++++++++++++------ module/abci.go | 74 ++++++++++++++++++++++++++++++++++---------------- 2 files changed, 81 insertions(+), 32 deletions(-) diff --git a/keeper/poa.go b/keeper/poa.go index d4495b4..346889b 100644 --- a/keeper/poa.go +++ b/keeper/poa.go @@ -72,6 +72,15 @@ func (k Keeper) SetPOAPower(ctx context.Context, valOpBech32 string, newShares i return stakingtypes.Validator{}, err } + // no need to process anything, same values + if newBFTConsensusPower == currentPower { + return val, fmt.Errorf("current power (%d) is the same as the new power (%d) for %s", currentPower, newBFTConsensusPower, valOpBech32) + } + + // When we SetValidatorByPowerIndex, the Tokens are used to get the shares of power for CometBFT consensus (voting_power). + // We don't `k.stakingKeeper.SetValidator` since we only use this for CometBFT consensus power. + val.Tokens = sdkmath.NewIntFromUint64(uint64(newShares)) + // slash all the validator's tokens (100%) if newShares == 0 && currentPower > 0 { pk, ok := val.ConsensusPubkey.GetCachedValue().(cryptotypes.PubKey) @@ -86,11 +95,25 @@ func (k Keeper) SetPOAPower(ctx context.Context, valOpBech32 string, newShares i if _, err := k.stakingKeeper.Slash(ctx, sdk.GetConsAddress(pk), height, normalizedToken.Int64(), sdkmath.LegacyOneDec()); err != nil { return stakingtypes.Validator{}, err } - } - // Sets the new consensus power for the validator (this is executed in the x/staking ApplyAndReturnValidatorUpdates method) - if err := k.stakingKeeper.SetLastValidatorPower(ctx, valAddr, newBFTConsensusPower); err != nil { - return stakingtypes.Validator{}, err + // TODO: delete the validator power index, or staking will handle? + + } else { + // Sets the new consensus power for the validator (this is executed in the x/staking ApplyAndReturnValidatorUpdates method) + if err := k.GetStakingKeeper().SetLastValidatorPower(ctx, valAddr, newBFTConsensusPower); err != nil { + return stakingtypes.Validator{}, err + } + if err := k.GetStakingKeeper().SetValidatorByPowerIndex(ctx, val); err != nil { + return stakingtypes.Validator{}, err + } + + // A cache to handle updated validators power. Once set, the next begin block will remove it from the cache. + // This allows us to Delete the validator index on the staking side, and ensures power updates do not persist over many blocks. + // This multi block persistance would break ABCI updates via x/staking if the validator's power is updated again, or removed. + if err := k.UpdatedValidatorsCache.Set(ctx, val.OperatorAddress); err != nil { + return stakingtypes.Validator{}, err + } + } absPowerDiff := uint64(math.Abs(float64(newBFTConsensusPower - currentPower))) @@ -129,6 +152,10 @@ func (k Keeper) AcceptNewValidator(ctx context.Context, operatingAddress string, return err } + if err := k.stakingKeeper.SetNewValidatorByPowerIndex(ctx, val); err != nil { + return err + } + // since the validator is set, remove it from the pending set if err := k.RemovePendingValidator(ctx, val.OperatorAddress); err != nil { return err @@ -172,10 +199,6 @@ func (k Keeper) setValidatorInternals(ctx context.Context, val stakingtypes.Vali return err } - if err := k.stakingKeeper.SetNewValidatorByPowerIndex(ctx, val); err != nil { - return err - } - return k.stakingKeeper.Hooks().AfterValidatorCreated(ctx, valAddr) } diff --git a/module/abci.go b/module/abci.go index c98890f..435327d 100644 --- a/module/abci.go +++ b/module/abci.go @@ -2,10 +2,10 @@ package module import ( "context" + "fmt" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/strangelove-ventures/poa" ) @@ -16,33 +16,44 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { sdkCtx := sdk.UnwrapSDKContext(ctx) defer telemetry.ModuleMeasureSince(poa.ModuleName, sdkCtx.BlockTime(), telemetry.MetricKeyBeginBlocker) - vals, err := am.keeper.GetStakingKeeper().GetAllValidators(ctx) + // iterate through any UpdatedValidatorsCache + iterator, err := am.keeper.UpdatedValidatorsCache.Iterate(ctx, nil) if err != nil { return err } + defer iterator.Close() - for _, v := range vals { - switch v.GetStatus() { - case stakingtypes.Unbonding: - // if the validator is unbonding, force it to be unbonded. (H+1) - v.Status = stakingtypes.Unbonded - if err := am.keeper.GetStakingKeeper().SetValidator(ctx, v); err != nil { - return err - } - - case stakingtypes.Unbonded: - // if the validator is unbonded (above case), delete the last validator power. (H+2) - valAddr, err := sdk.ValAddressFromBech32(v.OperatorAddress) - if err != nil { - return err - } - - if err := am.keeper.GetStakingKeeper().DeleteLastValidatorPower(ctx, valAddr); err != nil { - return err - } - - case stakingtypes.Unspecified, stakingtypes.Bonded: - continue + for ; iterator.Valid(); iterator.Next() { + valOperAddr, err := iterator.Key() + if err != nil { + return err + } + fmt.Printf("UpdatedValidatorsCache: %s\n", valOperAddr) + + sk := am.keeper.GetStakingKeeper() + + valAddr, err := sk.ValidatorAddressCodec().StringToBytes(valOperAddr) + if err != nil { + return err + } + + val, err := sk.GetValidator(ctx, valAddr) + if err != nil { + return err + } + + // TODO: needed? + // if err := k.stakingKeeper.DeleteLastValidatorPower(ctx, valAddr); err != nil { + // return stakingtypes.Validator{}, err + // } + + // Remove it from persisting across many blocks + if err := sk.DeleteValidatorByPowerIndex(ctx, val); err != nil { + return err + } + + if err := am.keeper.UpdatedValidatorsCache.Remove(ctx, valOperAddr); err != nil { + return err } } @@ -57,5 +68,20 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { } } + // get events + events, err := am.keeper.GetStakingKeeper().GetValidatorUpdates(ctx) + if err != nil { + return err + } + if len(events) == 0 { + return nil + } + + fmt.Println("\nBeginBlocker events:") + for _, e := range events { + fmt.Printf(" %s: %d\n", &e.PubKey, e.Power) + } + fmt.Println() + return nil } From 2168f45ac763ad1aa53cfb77db75519a8e5b2b5a Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 15:45:47 -0500 Subject: [PATCH 3/9] validate ABCI events are not stuck --- e2e/poa_add_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/e2e/poa_add_test.go b/e2e/poa_add_test.go index 4df990d..c465385 100644 --- a/e2e/poa_add_test.go +++ b/e2e/poa_add_test.go @@ -11,6 +11,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/strangelove-ventures/interchaintest/v8" "github.com/strangelove-ventures/interchaintest/v8/chain/cosmos" + "github.com/strangelove-ventures/interchaintest/v8/testutil" "github.com/strangelove-ventures/poa/e2e/helpers" "github.com/stretchr/testify/require" ) @@ -95,4 +96,10 @@ func TestPOAAddValidator(t *testing.T) { assertSignatures(t, ctx, chain, 1) // They are not signing, should be 0 assertConsensus(t, ctx, chain, 2) // ensure they were added to CometBFT + // validate that the ABCI events are not "stuck" (where the event is not cleared by POA or x/staking) + _, err = helpers.POASetPower(t, ctx, chain, admin, pending[0].OperatorAddress, 2_000_000) + require.NoError(t, err) + + require.NoError(t, testutil.WaitForBlocks(ctx, 4, chain)) + } From aef30cb62dd07a1b44c43bd0d5b0a77c3edcea6c Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 16:35:51 -0500 Subject: [PATCH 4/9] wait for blocks jail test --- e2e/poa_jail_test.go | 9 +++++++-- simapp/test_node.sh | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/e2e/poa_jail_test.go b/e2e/poa_jail_test.go index 0a72d81..8ebfe77 100644 --- a/e2e/poa_jail_test.go +++ b/e2e/poa_jail_test.go @@ -1,6 +1,7 @@ package e2e import ( + "fmt" "testing" "time" @@ -28,7 +29,7 @@ func TestPOAJailing(t *testing.T) { updatedSlashingCfg.ModifyGenesis = cosmos.ModifyGenesis(append(defaultGenesis, []cosmos.GenesisKV{ { Key: "app_state.slashing.params.signed_blocks_window", - Value: "3", + Value: "10", }, { Key: "app_state.slashing.params.min_signed_per_window", @@ -67,7 +68,7 @@ func TestPOAJailing(t *testing.T) { } // Wait for the stopped node to be jailed & persist - require.NoError(t, testutil.WaitForBlocks(ctx, 5, chain.Validators[0])) + require.NoError(t, testutil.WaitForBlocks(ctx, 15, chain.Validators[0])) // Validate 1 validator is jailed (status 1) vals := helpers.GetValidators(t, ctx, chain) @@ -75,6 +76,7 @@ func TestPOAJailing(t *testing.T) { require.True(t, func() bool { for _, v := range vals.Validators { if v.Status == int(stakingtypes.Unbonded) || v.Status == int(stakingtypes.Unbonding) { + fmt.Println("Validator", v.OperatorAddress, "is jailed", v.Status) jailedValAddr = v.OperatorAddress return true } @@ -94,4 +96,7 @@ func TestPOAJailing(t *testing.T) { require.True(t, unjailTime.After(now)) } } + + // wait for 2 blocks + require.NoError(t, testutil.WaitForBlocks(ctx, 2, chain)) } diff --git a/simapp/test_node.sh b/simapp/test_node.sh index 54b2512..0e4e66f 100644 --- a/simapp/test_node.sh +++ b/simapp/test_node.sh @@ -118,6 +118,11 @@ from_scratch () { # staking update_test_genesis '.app_state["staking"]["params"]["bond_denom"]="stake"' update_test_genesis '.app_state["staking"]["params"]["min_commission_rate"]="0.050000000000000000"' + # slashing + update_test_genesis '.app_state["slashing"]["params"]["signed_blocks_window"]="10"' + update_test_genesis '.app_state["slashing"]["params"]["min_signed_per_window"]="1.000000000000000000"' + update_test_genesis '.app_state["slashing"]["params"]["slash_fraction_double_sign"]="0.000000000000000000"' # no need to slash, no stake + update_test_genesis '.app_state["slashing"]["params"]["slash_fraction_downtime"]="0.000000000000000000"' # mint update_test_genesis '.app_state["mint"]["params"]["mint_denom"]="stake"' # crisis From 90e38470c45a5e63a9b22008221bbe0063506278 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 17:09:41 -0500 Subject: [PATCH 5/9] BeforeJailedValidatorsKey --- keeper/keeper.go | 2 + keeper/poa.go | 14 ++++++- keeper/staking_hooks.go | 84 +++++++++++++++++++++++++++++++++++++++++ keys.go | 2 + module/abci.go | 79 ++++++++++++++++++++++++++++++++++---- module/module.go | 5 +++ simapp/app.go | 10 +++-- 7 files changed, 183 insertions(+), 13 deletions(-) create mode 100644 keeper/staking_hooks.go diff --git a/keeper/keeper.go b/keeper/keeper.go index 636bcaa..d96be96 100644 --- a/keeper/keeper.go +++ b/keeper/keeper.go @@ -34,6 +34,7 @@ type Keeper struct { Params collections.Item[poa.Params] PendingValidators collections.Item[poa.Validators] UpdatedValidatorsCache collections.KeySet[string] + BeforeJailedValidators collections.KeySet[string] CachedBlockPower collections.Item[poa.PowerCache] AbsoluteChangedInBlockPower collections.Item[poa.PowerCache] @@ -63,6 +64,7 @@ func NewKeeper( Params: collections.NewItem(sb, poa.ParamsKey, "params", codec.CollValue[poa.Params](cdc)), PendingValidators: collections.NewItem(sb, poa.PendingValidatorsKey, "pending", codec.CollValue[poa.Validators](cdc)), UpdatedValidatorsCache: collections.NewKeySet(sb, poa.UpdatedValidatorsCacheKey, "updated_validators", collections.StringKey), + BeforeJailedValidators: collections.NewKeySet(sb, poa.BeforeJailedValidatorsKey, "before_jailed", collections.StringKey), CachedBlockPower: collections.NewItem(sb, poa.CachedPreviousBlockPowerKey, "cached_block", codec.CollValue[poa.PowerCache](cdc)), AbsoluteChangedInBlockPower: collections.NewItem(sb, poa.AbsoluteChangedInBlockPowerKey, "absolute_changed_power", codec.CollValue[poa.PowerCache](cdc)), diff --git a/keeper/poa.go b/keeper/poa.go index 346889b..6aaf7e1 100644 --- a/keeper/poa.go +++ b/keeper/poa.go @@ -88,6 +88,8 @@ func (k Keeper) SetPOAPower(ctx context.Context, valOpBech32 string, newShares i return stakingtypes.Validator{}, fmt.Errorf("issue getting consensus pubkey for %s", valOpBech32) } + consAddr := sdk.GetConsAddress(pk) + height := sdk.UnwrapSDKContext(ctx).BlockHeight() normalizedToken := k.stakingKeeper.TokensFromConsensusPower(ctx, currentPower) @@ -95,8 +97,16 @@ func (k Keeper) SetPOAPower(ctx context.Context, valOpBech32 string, newShares i if _, err := k.stakingKeeper.Slash(ctx, sdk.GetConsAddress(pk), height, normalizedToken.Int64(), sdkmath.LegacyOneDec()); err != nil { return stakingtypes.Validator{}, err } - - // TODO: delete the validator power index, or staking will handle? + // TODO: + if err := k.stakingKeeper.DeleteLastValidatorPower(ctx, valAddr); err != nil { + return stakingtypes.Validator{}, err + } + if err := k.stakingKeeper.DeleteValidatorByPowerIndex(ctx, val); err != nil { + return stakingtypes.Validator{}, err + } + if err := k.slashKeeper.DeleteMissedBlockBitmap(ctx, consAddr); err != nil { + return stakingtypes.Validator{}, err + } } else { // Sets the new consensus power for the validator (this is executed in the x/staking ApplyAndReturnValidatorUpdates method) diff --git a/keeper/staking_hooks.go b/keeper/staking_hooks.go new file mode 100644 index 0000000..1a17969 --- /dev/null +++ b/keeper/staking_hooks.go @@ -0,0 +1,84 @@ +package keeper + +import ( + "context" + "fmt" + + "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +// Before a validator is jailed, we must delete it from the power index. else: +// - ERR CONSENSUS FAILURE!!! err="should never retrieve a jailed validator from the power store" + +var _ stakingtypes.StakingHooks = Hooks{} + +// Hooks wrapper struct for staking keeper +type Hooks struct { + k Keeper +} + +// Hooks return the mesh-security hooks +func (k Keeper) Hooks() Hooks { + return Hooks{k} +} + +// BeforeValidatorModified implements types.StakingHooks. +func (h Hooks) BeforeValidatorModified(ctx context.Context, valAddr types.ValAddress) error { + fmt.Println("BeforeValidatorModified", valAddr.String()) + return h.k.BeforeJailedValidators.Set(ctx, valAddr.String()) +} + +// BeforeValidatorSlashed implements types.StakingHooks. +func (h Hooks) BeforeValidatorSlashed(ctx context.Context, valAddr types.ValAddress, fraction math.LegacyDec) error { + fmt.Println("BeforeValidatorSlashed", valAddr.String(), fraction.String()) + return nil +} + +// ---------------------------- + +// AfterDelegationModified implements types.StakingHooks. +func (h Hooks) AfterDelegationModified(ctx context.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + return nil +} + +// AfterUnbondingInitiated implements types.StakingHooks. +func (h Hooks) AfterUnbondingInitiated(ctx context.Context, id uint64) error { + return nil +} + +// AfterValidatorBeginUnbonding implements types.StakingHooks. +func (h Hooks) AfterValidatorBeginUnbonding(ctx context.Context, consAddr types.ConsAddress, valAddr types.ValAddress) error { + return nil +} + +// AfterValidatorBonded implements types.StakingHooks. +func (h Hooks) AfterValidatorBonded(ctx context.Context, consAddr types.ConsAddress, valAddr types.ValAddress) error { + return nil +} + +// AfterValidatorCreated implements types.StakingHooks. +func (h Hooks) AfterValidatorCreated(ctx context.Context, valAddr types.ValAddress) error { + return nil +} + +// AfterValidatorRemoved implements types.StakingHooks. +func (h Hooks) AfterValidatorRemoved(ctx context.Context, consAddr types.ConsAddress, valAddr types.ValAddress) error { + return nil +} + +// BeforeDelegationCreated implements types.StakingHooks. +func (h Hooks) BeforeDelegationCreated(ctx context.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + return nil +} + +// BeforeDelegationRemoved implements types.StakingHooks. +func (h Hooks) BeforeDelegationRemoved(ctx context.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + return nil +} + +// BeforeDelegationSharesModified implements types.StakingHooks. +func (h Hooks) BeforeDelegationSharesModified(ctx context.Context, delAddr types.AccAddress, valAddr types.ValAddress) error { + return nil +} diff --git a/keys.go b/keys.go index c4f39ae..dcd8a1c 100644 --- a/keys.go +++ b/keys.go @@ -20,6 +20,8 @@ var ( // UpdatedValidatorsCacheKey tracks recently updated validators from SetPower. UpdatedValidatorsCacheKey = collections.NewPrefix(4) + + BeforeJailedValidatorsKey = collections.NewPrefix(5) ) const ( diff --git a/module/abci.go b/module/abci.go index 435327d..3bb9173 100644 --- a/module/abci.go +++ b/module/abci.go @@ -10,19 +10,35 @@ import ( "github.com/strangelove-ventures/poa" ) +func (am AppModule) EndBlocker(ctx context.Context) error { + sk := am.keeper.GetStakingKeeper() + + // Front running x/staking maturity ? + if err := sk.UnbondAllMatureValidators(ctx); err != nil { + return err + } + + if err := am.handleBeforeJailedValidators(ctx); err != nil { + return err + } + + return nil +} + // BeginBlocker updates the validator set without applying updates. // Since this module depends on staking, that module will `ApplyAndReturnValidatorSetUpdates` from x/staking. func (am AppModule) BeginBlocker(ctx context.Context) error { sdkCtx := sdk.UnwrapSDKContext(ctx) defer telemetry.ModuleMeasureSince(poa.ModuleName, sdkCtx.BlockTime(), telemetry.MetricKeyBeginBlocker) - // iterate through any UpdatedValidatorsCache iterator, err := am.keeper.UpdatedValidatorsCache.Iterate(ctx, nil) if err != nil { return err } defer iterator.Close() + sk := am.keeper.GetStakingKeeper() + for ; iterator.Valid(); iterator.Next() { valOperAddr, err := iterator.Key() if err != nil { @@ -30,8 +46,6 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { } fmt.Printf("UpdatedValidatorsCache: %s\n", valOperAddr) - sk := am.keeper.GetStakingKeeper() - valAddr, err := sk.ValidatorAddressCodec().StringToBytes(valOperAddr) if err != nil { return err @@ -42,11 +56,6 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { return err } - // TODO: needed? - // if err := k.stakingKeeper.DeleteLastValidatorPower(ctx, valAddr); err != nil { - // return stakingtypes.Validator{}, err - // } - // Remove it from persisting across many blocks if err := sk.DeleteValidatorByPowerIndex(ctx, val); err != nil { return err @@ -57,6 +66,7 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { } } + // reset caches if sdkCtx.BlockHeight() > 1 { // non gentx messages reset the cached block powers for IBC validations. if err := am.keeper.ResetCachedTotalPower(ctx); err != nil { @@ -85,3 +95,56 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { return nil } + +func (am AppModule) handleBeforeJailedValidators(ctx context.Context) error { + sk := am.keeper.GetStakingKeeper() + + iterator, err := am.keeper.BeforeJailedValidators.Iterate(ctx, nil) + if err != nil { + return err + } + defer iterator.Close() + + // Why? we don't want it in the store w/ the val state change in x/staking + for ; iterator.Valid(); iterator.Next() { + valOperAddr, err := iterator.Key() + if err != nil { + return err + } + fmt.Printf("EndBlocker BeforeJailedValidators: %s\n", valOperAddr) + + valAddr, err := sk.ValidatorAddressCodec().StringToBytes(valOperAddr) + if err != nil { + return err + } + + val, err := sk.GetValidator(ctx, valAddr) + if err != nil { + return err + } + + if err := sk.DeleteValidatorByPowerIndex(ctx, val); err != nil { + return err + } + + // TODO: If this is used here, it persist ABCI Updates. When removes, it looks like the validator gets slashed every block in x/staking? (when we do the hack and force set jailed = false) + // if err := sk.DeleteLastValidatorPower(ctx, valAddr); err != nil { + // return err + // } + + // !IMPORTANT HACK: Set validator from jailed to not jailed to see what happens + // Okay so this like kind of worked for a split second + // Issue: the validator keeps trying to be converted to a jailed validator every single block when x/staking is calling it + val.Jailed = false + if err := sk.SetValidator(ctx, val); err != nil { + return err + } + + // remove it from persisting + if err := am.keeper.BeforeJailedValidators.Remove(ctx, valOperAddr); err != nil { + return err + } + } + + return nil +} diff --git a/module/module.go b/module/module.go index 496b667..d5cd1c2 100644 --- a/module/module.go +++ b/module/module.go @@ -27,6 +27,7 @@ var ( _ module.AppModule = AppModule{} _ module.AppModuleGenesis = AppModule{} _ appmodule.HasBeginBlocker = AppModule{} + _ appmodule.HasEndBlocker = AppModule{} ) // ConsensusVersion defines the current module consensus version. @@ -132,3 +133,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw func (am AppModule) BeginBlock(ctx context.Context) error { return am.BeginBlocker(ctx) } + +func (am AppModule) EndBlock(ctx context.Context) error { + return am.EndBlocker(ctx) +} diff --git a/simapp/app.go b/simapp/app.go index f1a78d2..eba7433 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -326,7 +326,11 @@ func NewSimApp( // register the staking hooks // NOTE: stakingKeeper above is passed by reference, so that it will contain these hooks app.StakingKeeper.SetHooks( - stakingtypes.NewMultiStakingHooks(app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks()), + stakingtypes.NewMultiStakingHooks( + app.DistrKeeper.Hooks(), + app.SlashingKeeper.Hooks(), + app.POAKeeper.Hooks(), // TODO: Document / add to spawn + ), ) app.CircuitKeeper = circuitkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[circuittypes.StoreKey]), govModAddress, app.AccountKeeper.AddressCodec()) @@ -450,16 +454,16 @@ func NewSimApp( distrtypes.ModuleName, slashingtypes.ModuleName, evidencetypes.ModuleName, + poa.ModuleName, // TODO: has to be first stakingtypes.ModuleName, - poa.ModuleName, genutiltypes.ModuleName, authz.ModuleName, ) app.ModuleManager.SetOrderEndBlockers( crisistypes.ModuleName, govtypes.ModuleName, + poa.ModuleName, // TODO: pretty sure this has to go first so we can remove jailed before BlockValidatorUpdates stakingtypes.ModuleName, - poa.ModuleName, genutiltypes.ModuleName, feegrant.ModuleName, group.ModuleName, From 2c8c7edc0279d4446a18b752d7b21724fc74a4b5 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 17:55:52 -0500 Subject: [PATCH 6/9] lint + fix remove val test (check bonded) --- e2e/poa_remove_test.go | 18 +++++++++++++++++- keeper/poa.go | 4 +--- keeper/staking_hooks.go | 8 ++++---- module/abci.go | 11 ++++++----- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/e2e/poa_remove_test.go b/e2e/poa_remove_test.go index f253daf..956d7fc 100644 --- a/e2e/poa_remove_test.go +++ b/e2e/poa_remove_test.go @@ -1,6 +1,7 @@ package e2e import ( + "context" "fmt" "testing" @@ -8,6 +9,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/strangelove-ventures/interchaintest/v8" "github.com/strangelove-ventures/interchaintest/v8/chain/cosmos" "github.com/strangelove-ventures/interchaintest/v8/testutil" @@ -75,7 +78,7 @@ func TestPOARemoval(t *testing.T) { assertSignatures(t, ctx, chain, initialValidators-1) require.Equal(t, initialValidators-1, len(consensus.Validators), "BFT consensus should have one less validator") - require.Equal(t, initialValidators-1, len(vals), "Validators should have one less validator") + require.Equal(t, initialValidators-1, getBondedValidators(t, ctx, chain), "Bonded validators should have one less active validator") } func getValToRemove(t *testing.T, vals helpers.Validators, delegationAmt int64) string { @@ -90,3 +93,16 @@ func getValToRemove(t *testing.T, vals helpers.Validators, delegationAmt int64) return valToRemove } + +func getBondedValidators(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain) int { + vals := helpers.GetValidators(t, ctx, chain).Validators + + bonded := 0 + for _, v := range vals { + if v.Status == int(stakingtypes.Bonded) { + bonded++ + } + } + + return bonded +} diff --git a/keeper/poa.go b/keeper/poa.go index 6aaf7e1..ca43c9e 100644 --- a/keeper/poa.go +++ b/keeper/poa.go @@ -107,7 +107,6 @@ func (k Keeper) SetPOAPower(ctx context.Context, valOpBech32 string, newShares i if err := k.slashKeeper.DeleteMissedBlockBitmap(ctx, consAddr); err != nil { return stakingtypes.Validator{}, err } - } else { // Sets the new consensus power for the validator (this is executed in the x/staking ApplyAndReturnValidatorUpdates method) if err := k.GetStakingKeeper().SetLastValidatorPower(ctx, valAddr, newBFTConsensusPower); err != nil { @@ -119,11 +118,10 @@ func (k Keeper) SetPOAPower(ctx context.Context, valOpBech32 string, newShares i // A cache to handle updated validators power. Once set, the next begin block will remove it from the cache. // This allows us to Delete the validator index on the staking side, and ensures power updates do not persist over many blocks. - // This multi block persistance would break ABCI updates via x/staking if the validator's power is updated again, or removed. + // This multi block persistence would break ABCI updates via x/staking if the validator's power is updated again, or removed. if err := k.UpdatedValidatorsCache.Set(ctx, val.OperatorAddress); err != nil { return stakingtypes.Validator{}, err } - } absPowerDiff := uint64(math.Abs(float64(newBFTConsensusPower - currentPower))) diff --git a/keeper/staking_hooks.go b/keeper/staking_hooks.go index 1a17969..60c354e 100644 --- a/keeper/staking_hooks.go +++ b/keeper/staking_hooks.go @@ -2,11 +2,11 @@ package keeper import ( "context" - "fmt" - "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + "cosmossdk.io/math" ) // Before a validator is jailed, we must delete it from the power index. else: @@ -26,13 +26,13 @@ func (k Keeper) Hooks() Hooks { // BeforeValidatorModified implements types.StakingHooks. func (h Hooks) BeforeValidatorModified(ctx context.Context, valAddr types.ValAddress) error { - fmt.Println("BeforeValidatorModified", valAddr.String()) + h.k.logger.Info("BeforeValidatorModified", "valAddr", valAddr.String()) return h.k.BeforeJailedValidators.Set(ctx, valAddr.String()) } // BeforeValidatorSlashed implements types.StakingHooks. func (h Hooks) BeforeValidatorSlashed(ctx context.Context, valAddr types.ValAddress, fraction math.LegacyDec) error { - fmt.Println("BeforeValidatorSlashed", valAddr.String(), fraction.String()) + h.k.logger.Info("BeforeValidatorSlashed", valAddr.String(), fraction.String()) return nil } diff --git a/module/abci.go b/module/abci.go index 3bb9173..5c25364 100644 --- a/module/abci.go +++ b/module/abci.go @@ -44,7 +44,7 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { if err != nil { return err } - fmt.Printf("UpdatedValidatorsCache: %s\n", valOperAddr) + am.keeper.Logger().Info("UpdatedValidatorsCache: %s\n", valOperAddr) valAddr, err := sk.ValidatorAddressCodec().StringToBytes(valOperAddr) if err != nil { @@ -87,11 +87,12 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { return nil } - fmt.Println("\nBeginBlocker events:") + // TODO: Error here so it's easy to see (remove later) + am.keeper.Logger().Error("BeginBlocker events:\n") for _, e := range events { - fmt.Printf(" %s: %d\n", &e.PubKey, e.Power) + am.keeper.Logger().Error(fmt.Sprintf("PubKey: %s, Power: %d", &e.PubKey, e.Power)) } - fmt.Println() + am.keeper.Logger().Info("\n") return nil } @@ -111,7 +112,7 @@ func (am AppModule) handleBeforeJailedValidators(ctx context.Context) error { if err != nil { return err } - fmt.Printf("EndBlocker BeforeJailedValidators: %s\n", valOperAddr) + am.keeper.Logger().Info("EndBlocker BeforeJailedValidators", valOperAddr, "\n") valAddr, err := sk.ValidatorAddressCodec().StringToBytes(valOperAddr) if err != nil { From 8cddb62d8bfaa8b44aa2015069fce4b2191e1564 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 17:58:56 -0500 Subject: [PATCH 7/9] lint --- module/abci.go | 1 + 1 file changed, 1 insertion(+) diff --git a/module/abci.go b/module/abci.go index 5c25364..10f5d59 100644 --- a/module/abci.go +++ b/module/abci.go @@ -90,6 +90,7 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { // TODO: Error here so it's easy to see (remove later) am.keeper.Logger().Error("BeginBlocker events:\n") for _, e := range events { + e := e am.keeper.Logger().Error(fmt.Sprintf("PubKey: %s, Power: %d", &e.PubKey, e.Power)) } am.keeper.Logger().Info("\n") From d0a5c34768cbfab994f89ef5bdf799f9f7cef4ae Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 18:09:50 -0500 Subject: [PATCH 8/9] logs --- e2e/poa_jail_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/poa_jail_test.go b/e2e/poa_jail_test.go index 8ebfe77..2c4d811 100644 --- a/e2e/poa_jail_test.go +++ b/e2e/poa_jail_test.go @@ -68,6 +68,7 @@ func TestPOAJailing(t *testing.T) { } // Wait for the stopped node to be jailed & persist + t.Log("Waiting for validator to become jailed") require.NoError(t, testutil.WaitForBlocks(ctx, 15, chain.Validators[0])) // Validate 1 validator is jailed (status 1) @@ -97,6 +98,7 @@ func TestPOAJailing(t *testing.T) { } } - // wait for 2 blocks - require.NoError(t, testutil.WaitForBlocks(ctx, 2, chain)) + // wait to ensure the chain is not halted + t.Log("Waiting for chain to progress") + require.NoError(t, testutil.WaitForBlocks(ctx, 5, chain)) } From 775d3ad34523bb1a6068ff79ae3e645eabd02a6c Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Sun, 14 Apr 2024 18:13:33 -0500 Subject: [PATCH 9/9] touchups --- INTEGRATION.md | 11 +++++++++++ module/abci.go | 7 +++---- simapp/app.go | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/INTEGRATION.md b/INTEGRATION.md index 1a855ca..dd93574 100644 --- a/INTEGRATION.md +++ b/INTEGRATION.md @@ -71,6 +71,16 @@ app.ModuleManager = module.NewManager( ... +// Register POA Staking Hooks +app.StakingKeeper.SetHooks( + stakingtypes.NewMultiStakingHooks( + ... + app.POAKeeper.Hooks(), + ), +) + +... + // Add PoA to BeginBlock logic // NOTE: This must be before the staking module begin blocker app.ModuleManager.SetOrderBeginBlockers( @@ -80,6 +90,7 @@ app.ModuleManager.SetOrderBeginBlockers( ) // Add PoA to end blocker logic +// NOTE: This must be before the staking module end blocker app.ModuleManager.SetOrderEndBlockers( ... poa.ModuleName, diff --git a/module/abci.go b/module/abci.go index 10f5d59..961770b 100644 --- a/module/abci.go +++ b/module/abci.go @@ -78,7 +78,7 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { } } - // get events + // Event Debugging events, err := am.keeper.GetStakingKeeper().GetValidatorUpdates(ctx) if err != nil { return err @@ -87,11 +87,10 @@ func (am AppModule) BeginBlocker(ctx context.Context) error { return nil } - // TODO: Error here so it's easy to see (remove later) - am.keeper.Logger().Error("BeginBlocker events:\n") + am.keeper.Logger().Info("BeginBlocker events:\n") for _, e := range events { e := e - am.keeper.Logger().Error(fmt.Sprintf("PubKey: %s, Power: %d", &e.PubKey, e.Power)) + am.keeper.Logger().Info(fmt.Sprintf("PubKey: %s, Power: %d", &e.PubKey, e.Power)) } am.keeper.Logger().Info("\n") diff --git a/simapp/app.go b/simapp/app.go index eba7433..42b2fa5 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -329,7 +329,7 @@ func NewSimApp( stakingtypes.NewMultiStakingHooks( app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks(), - app.POAKeeper.Hooks(), // TODO: Document / add to spawn + app.POAKeeper.Hooks(), ), )