From 9f446eb180ad83457ce3e228f96ccca3313d9af3 Mon Sep 17 00:00:00 2001 From: jinmannwong <38482272+jinmannwong@users.noreply.github.com> Date: Tue, 11 Aug 2020 12:23:21 +0100 Subject: [PATCH] Separate dkg and consensus validator updates for overlapping dkgs (#34) * Refactor delaying of staking updates * Remove print statements * Remove turning off of update delays * Add comments * Review feedback * Revert validator key as func back to var * Split dkg and consensus validator updates * Bug fix on dkg validator updates * Bump consensus to v0.6.0 * Modify comments * Modify comments * Modify comment --- types/module/module.go | 8 +-- x/distribution/keeper/delegation_test.go | 5 +- x/distribution/keeper/test_common.go | 1 + x/evidence/internal/keeper/infraction_test.go | 15 ++-- x/gov/abci_test.go | 2 +- x/gov/keeper/test_common.go | 11 +-- x/gov/test_common.go | 7 -- x/slashing/abci_test.go | 5 +- x/slashing/app_test.go | 12 +--- x/slashing/handler_test.go | 16 ++--- x/slashing/internal/keeper/keeper_test.go | 21 +++--- x/slashing/internal/keeper/test_common.go | 8 +-- x/staking/abci.go | 8 +-- x/staking/app_test.go | 7 +- x/staking/handler_test.go | 71 ++++++++----------- x/staking/keeper/delay_updates.go | 50 +++++++++---- x/staking/keeper/keeper.go | 35 +++++---- x/staking/keeper/test_common.go | 1 + x/staking/test_common.go | 8 --- 19 files changed, 133 insertions(+), 158 deletions(-) diff --git a/types/module/module.go b/types/module/module.go index 6747d16e4c0..246d7d6035f 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -315,8 +315,8 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo // use these validator updates if provided, the module manager assumes // only one module will update the validator set and dkg validator set - if len(moduleValUpdates) > 0 { - if len(validatorUpdates) > 0 { + if len(moduleValUpdates) > 0 || len(moduleDKGValUpdates) > 0 { + if len(validatorUpdates) > 0 || len(dkgValidatorUpdates) > 0 { panic("validator EndBlock updates already set by a previous module") } @@ -326,8 +326,8 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo } return abci.ResponseEndBlock{ - ValidatorUpdates: validatorUpdates, + ValidatorUpdates: validatorUpdates, DkgValidatorUpdates: dkgValidatorUpdates, - Events: ctx.EventManager().ABCIEvents(), + Events: ctx.EventManager().ABCIEvents(), } } diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index edc5240f06a..666a0be4b7c 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -543,8 +543,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { require.NotNil(t, res) // end block to bond validator - header := abci.Header{Entropy: abci.BlockEntropy{Round: 1, AeonLength: 3}} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // next block @@ -574,7 +573,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { del2 := sk.Delegation(ctx, sdk.AccAddress(valOpAddr2), valOpAddr1) // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // next block diff --git a/x/distribution/keeper/test_common.go b/x/distribution/keeper/test_common.go index 78425e47c11..43bb5c0f86f 100644 --- a/x/distribution/keeper/test_common.go +++ b/x/distribution/keeper/test_common.go @@ -78,6 +78,7 @@ func CreateTestInputDefault(t *testing.T, isCheckTx bool, initPower int64) ( communityTax := sdk.NewDecWithPrec(2, 2) ctx, ak, _, dk, sk, _, supplyKeeper := CreateTestInputAdvanced(t, isCheckTx, initPower, communityTax) + sk.SetDelayValidatorUpdates(false) return ctx, ak, dk, sk, supplyKeeper } diff --git a/x/evidence/internal/keeper/infraction_test.go b/x/evidence/internal/keeper/infraction_test.go index 21884722590..1961fc1214a 100644 --- a/x/evidence/internal/keeper/infraction_test.go +++ b/x/evidence/internal/keeper/infraction_test.go @@ -22,6 +22,7 @@ func newTestMsgCreateValidator(address sdk.ValAddress, pubKey crypto.PubKey, amt func (suite *KeeperTestSuite) TestHandleDoubleSign() { ctx := suite.ctx.WithIsCheckTx(false).WithBlockHeight(1) suite.populateValidators(ctx) + suite.app.StakingKeeper.SetDelayValidatorUpdates(false) power := int64(100) stakingParams := suite.app.StakingKeeper.GetParams(ctx) @@ -34,8 +35,7 @@ func (suite *KeeperTestSuite) TestHandleDoubleSign() { suite.NotNil(res) // execute end-blocker and verify validator attributes - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, suite.app.StakingKeeper) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, suite.app.StakingKeeper) staking.EndBlocker(ctx, suite.app.StakingKeeper) suite.Equal( suite.app.BankKeeper.GetCoins(ctx, sdk.AccAddress(operatorAddr)), @@ -90,6 +90,7 @@ func (suite *KeeperTestSuite) TestHandleDoubleSign() { func (suite *KeeperTestSuite) TestHandleDoubleSign_TooOld() { ctx := suite.ctx.WithIsCheckTx(false).WithBlockHeight(1).WithBlockTime(time.Now()) suite.populateValidators(ctx) + suite.app.StakingKeeper.SetDelayValidatorUpdates(false) power := int64(100) stakingParams := suite.app.StakingKeeper.GetParams(ctx) @@ -102,8 +103,7 @@ func (suite *KeeperTestSuite) TestHandleDoubleSign_TooOld() { suite.NotNil(res) // execute end-blocker and verify validator attributes - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, suite.app.StakingKeeper) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, suite.app.StakingKeeper) staking.EndBlocker(ctx, suite.app.StakingKeeper) suite.Equal( suite.app.BankKeeper.GetCoins(ctx, sdk.AccAddress(operatorAddr)), @@ -123,10 +123,3 @@ func (suite *KeeperTestSuite) TestHandleDoubleSign_TooOld() { suite.False(suite.app.StakingKeeper.Validator(ctx, operatorAddr).IsJailed()) suite.False(suite.app.SlashingKeeper.IsTombstoned(ctx, sdk.ConsAddress(val.Address()))) } - -func testBlockEntropy() abci.BlockEntropy { - return abci.BlockEntropy{ - Round: 1, - AeonLength: 3, - } -} diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index 137a26db82c..db5509f45e1 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -252,7 +252,7 @@ func TestProposalPassedEndblocker(t *testing.T) { handler := NewHandler(input.keeper) stakingHandler := staking.NewHandler(input.sk) - header := abci.Header{Height: input.mApp.LastBlockHeight() + 1, Entropy: testBlockEntropy()} + header := abci.Header{Height: input.mApp.LastBlockHeight() + 1} input.mApp.BeginBlock(abci.RequestBeginBlock{Header: header}) ctx := input.mApp.BaseApp.NewContext(false, abci.Header{}) diff --git a/x/gov/keeper/test_common.go b/x/gov/keeper/test_common.go index b9d4ecdaac5..926d310aed6 100644 --- a/x/gov/keeper/test_common.go +++ b/x/gov/keeper/test_common.go @@ -146,6 +146,7 @@ func createTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context sk := staking.NewKeeper(cdc, keyStaking, supplyKeeper, pk.Subspace(staking.DefaultParamspace)) sk.SetParams(ctx, staking.DefaultParams()) + sk.SetDelayValidatorUpdates(false) rtr := types.NewRouter(). AddRoute(types.RouterKey, types.ProposalHandler) @@ -201,14 +202,6 @@ func createValidators(ctx sdk.Context, sk staking.Keeper, powers []int64) { _, _ = sk.Delegate(ctx, valAccAddr2, sdk.TokensFromConsensusPower(powers[1]), sdk.Unbonded, val2, true) _, _ = sk.Delegate(ctx, valAccAddr3, sdk.TokensFromConsensusPower(powers[2]), sdk.Unbonded, val3, true) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) } - -func testBlockEntropy() abci.BlockEntropy { - return abci.BlockEntropy{ - Round: 1, - AeonLength: 3, - } -} diff --git a/x/gov/test_common.go b/x/gov/test_common.go index 9b08fb9347a..7ecd4bd50f6 100644 --- a/x/gov/test_common.go +++ b/x/gov/test_common.go @@ -237,10 +237,3 @@ func createValidators(t *testing.T, stakingHandler sdk.Handler, ctx sdk.Context, require.NotNil(t, res) } } - -func testBlockEntropy() abci.BlockEntropy { - return abci.BlockEntropy{ - Round: 1, - AeonLength: 3, - } -} diff --git a/x/slashing/abci_test.go b/x/slashing/abci_test.go index eee3c3b6296..3d9c17a212f 100644 --- a/x/slashing/abci_test.go +++ b/x/slashing/abci_test.go @@ -24,8 +24,7 @@ func TestBeginBlocker(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) require.Equal( t, ck.GetCoins(ctx, sdk.AccAddress(addr)), @@ -87,7 +86,7 @@ func TestBeginBlocker(t *testing.T) { } // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // validator should be jailed diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index 1ab703dfe2e..b26c1d06788 100644 --- a/x/slashing/app_test.go +++ b/x/slashing/app_test.go @@ -56,6 +56,7 @@ func getMockApp(t *testing.T) (*mock.App, staking.Keeper, Keeper) { } supplyKeeper := supply.NewKeeper(mapp.Cdc, keySupply, mapp.AccountKeeper, bankKeeper, maccPerms) stakingKeeper := staking.NewKeeper(mapp.Cdc, keyStaking, supplyKeeper, mapp.ParamsKeeper.Subspace(staking.DefaultParamspace)) + stakingKeeper.SetDelayValidatorUpdates(false) keeper := NewKeeper(mapp.Cdc, keySlashing, stakingKeeper, mapp.ParamsKeeper.Subspace(DefaultParamspace)) mapp.Router().AddRoute(staking.RouterKey, staking.NewHandler(stakingKeeper)) mapp.Router().AddRoute(RouterKey, NewHandler(keeper)) @@ -144,11 +145,11 @@ func TestSlashingMsgs(t *testing.T) { sdk.ValAddress(addr1), priv1.PubKey(), bondCoin, description, commission, sdk.OneInt(), ) - header := abci.Header{Height: mapp.LastBlockHeight() + 1, Entropy: testBlockEntropy()} + header := abci.Header{Height: mapp.LastBlockHeight() + 1} mock.SignCheckDeliver(t, mapp.Cdc, mapp.BaseApp, header, []sdk.Msg{createValidatorMsg}, []uint64{0}, []uint64{0}, true, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{genCoin.Sub(bondCoin)}) - header = abci.Header{Height: mapp.LastBlockHeight() + 1, Entropy: testBlockEntropy()} + header = abci.Header{Height: mapp.LastBlockHeight() + 1} mapp.BeginBlock(abci.RequestBeginBlock{Header: header}) validator := checkValidator(t, mapp, stakingKeeper, addr1, true) @@ -167,10 +168,3 @@ func TestSlashingMsgs(t *testing.T) { require.Nil(t, res) require.True(t, errors.Is(ErrValidatorNotJailed, err)) } - -func testBlockEntropy() abci.BlockEntropy { - return abci.BlockEntropy{ - Round: 1, - AeonLength: 3, - } -} diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 0c6b770a753..852313283ba 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -27,8 +27,7 @@ func TestCannotUnjailUnlessJailed(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) require.Equal( @@ -168,8 +167,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) require.Equal( @@ -223,7 +221,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.Equal(t, int64(0), info.MissedBlocksCounter) // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // validator should have been jailed @@ -245,7 +243,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.Equal(t, int64(1), info.MissedBlocksCounter) // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // validator should not have been slashed any more, since it was already jailed @@ -264,7 +262,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.NotNil(t, res) // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // validator should be rebonded now @@ -297,7 +295,7 @@ func TestHandleAbsentValidator(t *testing.T) { } // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // validator should be jailed again after 500 unsigned blocks @@ -308,7 +306,7 @@ func TestHandleAbsentValidator(t *testing.T) { } // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) diff --git a/x/slashing/internal/keeper/keeper_test.go b/x/slashing/internal/keeper/keeper_test.go index 89520027e15..62005e3deeb 100644 --- a/x/slashing/internal/keeper/keeper_test.go +++ b/x/slashing/internal/keeper/keeper_test.go @@ -30,8 +30,7 @@ func TestHandleNewValidator(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) require.Equal( @@ -74,8 +73,7 @@ func TestHandleAlreadyJailed(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // 1000 first blocks OK @@ -92,7 +90,7 @@ func TestHandleAlreadyJailed(t *testing.T) { } // end block - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // validator should have been jailed and slashed @@ -133,8 +131,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) // 100 first blocks OK @@ -150,7 +147,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) validatorUpdates, _ := staking.EndBlocker(ctx, sk) require.Equal(t, 2, len(validatorUpdates)) validator, _ := sk.GetValidator(ctx, addr) @@ -166,7 +163,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) validatorUpdates, _ = staking.EndBlocker(ctx, sk) require.Equal(t, 2, len(validatorUpdates)) validator, _ = sk.GetValidator(ctx, addr) @@ -189,7 +186,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { } // should now be jailed & kicked - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) validator, _ = sk.GetValidator(ctx, addr) require.Equal(t, sdk.Unbonding, validator.Status) @@ -215,7 +212,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { height++ // validator should not be kicked since we reset counter/array when it was jailed - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) validator, _ = sk.GetValidator(ctx, addr) require.Equal(t, sdk.Bonded, validator.Status) @@ -228,7 +225,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { } // validator should now be jailed & kicked - staking.BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, sk) + staking.BeginBlocker(ctx, abci.RequestBeginBlock{}, sk) staking.EndBlocker(ctx, sk) validator, _ = sk.GetValidator(ctx, addr) require.Equal(t, sdk.Unbonding, validator.Status) diff --git a/x/slashing/internal/keeper/test_common.go b/x/slashing/internal/keeper/test_common.go index 9a5056e8c16..3293d9607d8 100644 --- a/x/slashing/internal/keeper/test_common.go +++ b/x/slashing/internal/keeper/test_common.go @@ -103,6 +103,7 @@ func CreateTestInput(t *testing.T, defaults types.Params) (sdk.Context, bank.Kee supplyKeeper.SetSupply(ctx, supply.NewSupply(totalSupply)) sk := staking.NewKeeper(cdc, keyStaking, supplyKeeper, paramsKeeper.Subspace(staking.DefaultParamspace)) + sk.SetDelayValidatorUpdates(false) genesis := staking.DefaultGenesisState() // set module accounts @@ -156,10 +157,3 @@ func NewTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, delAmoun amount := sdk.NewCoin(sdk.DefaultBondDenom, delAmount) return staking.NewMsgDelegate(delAddr, valAddr, amount) } - -func testBlockEntropy() abci.BlockEntropy { - return abci.BlockEntropy{ - Round: 1, - AeonLength: 3, - } -} diff --git a/x/staking/abci.go b/x/staking/abci.go index 97e5bc49a88..40dbce6a1fc 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -16,8 +16,8 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) // Called every block, update validator set func EndBlocker(ctx sdk.Context, k keeper.Keeper) ([]abci.ValidatorUpdate, []abci.ValidatorUpdate) { - if !k.PerformValidatorUpdates(ctx) { - return []abci.ValidatorUpdate{}, []abci.ValidatorUpdate{} - } - return k.BlockValidatorUpdates(ctx), []abci.ValidatorUpdate{} + // If dkg and validator updates are triggered at the same time then dkg validator updates + // must be computed first + dkgUpdates := k.DKGValidatorUpdates(ctx) + return k.ValidatorUpdates(ctx), dkgUpdates } diff --git a/x/staking/app_test.go b/x/staking/app_test.go index 121d91f8eeb..f23b068e3a1 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -43,6 +43,7 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { } supplyKeeper := supply.NewKeeper(mApp.Cdc, keySupply, mApp.AccountKeeper, bankKeeper, maccPerms) keeper := NewKeeper(mApp.Cdc, keyStaking, supplyKeeper, mApp.ParamsKeeper.Subspace(DefaultParamspace)) + keeper.SetDelayValidatorUpdates(false) mApp.Router().AddRoute(RouterKey, NewHandler(keeper)) mApp.SetBeginBlocker(getBeginBlocker(keeper)) @@ -68,7 +69,7 @@ func getEndBlocker(keeper Keeper) sdk.EndBlocker { validatorUpdates, dkgValidatorUpdates := EndBlocker(ctx, keeper) return abci.ResponseEndBlock{ - ValidatorUpdates: validatorUpdates, + ValidatorUpdates: validatorUpdates, DkgValidatorUpdates: dkgValidatorUpdates, } } @@ -151,11 +152,11 @@ func TestStakingMsgs(t *testing.T) { sdk.ValAddress(addr1), priv1.PubKey(), bondCoin, description, commissionRates, sdk.OneInt(), ) - header := abci.Header{Height: mApp.LastBlockHeight() + 1, Entropy: testBlockEntropy()} + header := abci.Header{Height: mApp.LastBlockHeight() + 1} mock.SignCheckDeliver(t, mApp.Cdc, mApp.BaseApp, header, []sdk.Msg{createValidatorMsg}, []uint64{0}, []uint64{0}, true, true, priv1) mock.CheckBalance(t, mApp, addr1, sdk.Coins{genCoin.Sub(bondCoin)}) - header = abci.Header{Height: mApp.LastBlockHeight() + 1, Entropy: testBlockEntropy()} + header = abci.Header{Height: mApp.LastBlockHeight() + 1} mApp.BeginBlock(abci.RequestBeginBlock{Header: header}) validator := checkValidator(t, mApp, keeper, sdk.ValAddress(addr1), true) diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index 9f9174fba8b..b61b4971969 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -94,8 +94,7 @@ func TestValidatorByPowerIndex(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(res.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // verify that by power key nolonger exists @@ -464,8 +463,7 @@ func TestIncrementsMsgUnbond(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(res.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // check that the accounts and the bond account have the appropriate values @@ -562,8 +560,7 @@ func TestMultipleMsgCreateValidator(t *testing.T) { require.Equal(t, balanceExpd, balanceGot, "expected account %d to have %d, got %d", i, balanceExpd, balanceGot) } - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // unbond them all by removing delegation @@ -581,11 +578,11 @@ func TestMultipleMsgCreateValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(res.Data, &finishTime) // adds validator into unbonding queue - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // removes validator from queue and set - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)), keeper) // Check that the validator is deleted from state @@ -718,8 +715,7 @@ func TestValidatorQueue(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // unbond the all self-delegation to put validator in unbonding state @@ -733,7 +729,7 @@ func TestValidatorQueue(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(res.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) origHeader := ctx.BlockHeader() @@ -744,7 +740,7 @@ func TestValidatorQueue(t *testing.T) { // should still be unbonding at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) validator, found = keeper.GetValidator(ctx, validatorAddr) @@ -753,7 +749,7 @@ func TestValidatorQueue(t *testing.T) { // should be in unbonded state at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) validator, found = keeper.GetValidator(ctx, validatorAddr) @@ -777,8 +773,7 @@ func TestUnbondingPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // begin unbonding @@ -794,21 +789,21 @@ func TestUnbondingPeriod(t *testing.T) { require.True(t, found, "should not have unbonded") // cannot complete unbonding at same time - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.True(t, found, "should not have unbonded") // cannot complete unbonding at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.True(t, found, "should not have unbonded") // can complete unbonding at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.False(t, found, "should have unbonded") @@ -851,8 +846,7 @@ func TestUnbondingFromUnbondingValidator(t *testing.T) { ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(keeper.UnbondingTime(ctx))) // Run the EndBlocker - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // Check to make sure that the unbonding delegation is no longer in state @@ -906,22 +900,21 @@ func TestRedelegationPeriod(t *testing.T) { origHeader := ctx.BlockHeader() // cannot complete redelegation at same time - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) _, found := keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.True(t, found, "should not have unbonded") // cannot complete redelegation at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) _, found = keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.True(t, found, "should not have unbonded") // can complete redelegation at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) _, found = keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.False(t, found, "should have unbonded") @@ -969,8 +962,7 @@ func TestTransitiveRedelegation(t *testing.T) { ctx = ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)) // complete first redelegation - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // now should be able to redelegate from the second validator to the third @@ -1002,8 +994,7 @@ func TestMultipleRedelegationAtSameTime(t *testing.T) { require.NotNil(t, res) // end block to bond them - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // begin a redelegate @@ -1031,7 +1022,7 @@ func TestMultipleRedelegationAtSameTime(t *testing.T) { // move forward in time, should complete both redelegations ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(1 * time.Second)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) @@ -1061,8 +1052,7 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { require.NotNil(t, res) // end block to bond them - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // begin a redelegate @@ -1086,7 +1076,7 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { // move forward in time, should complete the first redelegation, but not the second ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.True(t, found) @@ -1094,7 +1084,7 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { // move forward in time, should complete the second redelegation ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.False(t, found) @@ -1117,8 +1107,7 @@ func TestMultipleUnbondingDelegationAtSameTime(t *testing.T) { require.NotNil(t, res) // end block to bond - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // begin an unbonding delegation @@ -1146,7 +1135,7 @@ func TestMultipleUnbondingDelegationAtSameTime(t *testing.T) { // move forwaubd in time, should complete both ubds ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(1 * time.Second)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) @@ -1170,8 +1159,7 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { require.NotNil(t, res) // end block to bond - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // begin an unbonding delegation @@ -1200,7 +1188,7 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { // move forwaubd in time, should complete the first redelegation, but not the second ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.True(t, found) @@ -1208,7 +1196,7 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { // move forwaubd in time, should complete the second redelegation ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.False(t, found) @@ -1373,8 +1361,7 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) { require.Equal(t, sdk.NewDecFromInt(redAmt.Amount.QuoRaw(2)), delegation.Shares) // end blocker - header := abci.Header{Entropy: testBlockEntropy()} - BeginBlocker(ctx, abci.RequestBeginBlock{Header: header}, keeper) + BeginBlocker(ctx, abci.RequestBeginBlock{}, keeper) EndBlocker(ctx, keeper) // validator power should have been reduced to zero diff --git a/x/staking/keeper/delay_updates.go b/x/staking/keeper/delay_updates.go index 0d0ffe503f0..7498b8489e2 100644 --- a/x/staking/keeper/delay_updates.go +++ b/x/staking/keeper/delay_updates.go @@ -7,24 +7,50 @@ import ( ) var ( - validatorUpdateKey = []byte("validatorUpdateKey") + computeValidatorUpdateKey = []byte("computeValidatorUpdateKey") + computeDKGValidatorUpdateKey = []byte("computeDKGValidatorUpdateKey") + validatorUpdatesKey = []byte("validatorUpdatesKey") ) -// Check whether entropy generation round corresponds to validator changeover height +// CheckValidatorUpdates determines whether block height is sufficiently close to the next aeon start +// to trigger dkg and consensus validator changes func (k Keeper) CheckValidatorUpdates(ctx sdk.Context, header abci.Header) { - // If two blocks before an next aeon start, need to return new validator set of next aeon - if header.Entropy.GetRound() == header.Entropy.GetAeonLength()-2 { + // One block before a new aeon start need to compute validator updates for next dkg committee. + // Two blocks before a new aeon start need to update consensus committee to those which ran dkg + nextAeonStart := header.Entropy.NextAeonStart + if !k.delayValidatorUpdates || header.Height == nextAeonStart-1 { store := ctx.KVStore(k.storeKey) - store.Set(validatorUpdateKey, []byte{0}) + store.Set(computeDKGValidatorUpdateKey, []byte{0}) + } + if !k.delayValidatorUpdates || header.Height == nextAeonStart-2 { + store := ctx.KVStore(k.storeKey) + store.Set(computeValidatorUpdateKey, []byte{0}) } } -// Tells EndBlocker whether to compute validator updates using variable set in BeginBlocker -func (k Keeper) PerformValidatorUpdates(ctx sdk.Context) bool { +// DKGValidatorUpdates returns dkg validator updates to EndBlock at block height set by BeginBlock and +// saves them to store for retrieval by ValidatorUpdates +func (k Keeper) DKGValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate { store := ctx.KVStore(k.storeKey) - if len(store.Get(validatorUpdateKey)) == 0 { - return false + if len(store.Get(computeDKGValidatorUpdateKey)) == 0 { + return []abci.ValidatorUpdate{} } - store.Set(validatorUpdateKey, []byte{}) - return true -} \ No newline at end of file + store.Set(computeDKGValidatorUpdateKey, []byte{}) + updates := k.BlockValidatorUpdates(ctx) + store.Set(validatorUpdatesKey, k.cdc.MustMarshalBinaryLengthPrefixed(updates)) + return updates +} + +// ValidatorUpdates retrieve last saved updates from store when non-trivial update +// is triggered by BeginBlock, +func (k Keeper) ValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate { + store := ctx.KVStore(k.storeKey) + if len(store.Get(computeValidatorUpdateKey)) == 0 { + return []abci.ValidatorUpdate{} + } + store.Set(computeValidatorUpdateKey, []byte{}) + updateBytes := store.Get(validatorUpdatesKey) + updates := []abci.ValidatorUpdate{} + k.cdc.UnmarshalBinaryLengthPrefixed(updateBytes, &updates) + return updates +} diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index fd51603115b..b920684e959 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -22,13 +22,14 @@ var _ types.DelegationSet = Keeper{} // keeper of the staking store type Keeper struct { - storeKey sdk.StoreKey - cdc *codec.Codec - supplyKeeper types.SupplyKeeper - hooks types.StakingHooks - paramstore params.Subspace - validatorCache map[string]cachedValidator - validatorCacheList *list.List + storeKey sdk.StoreKey + cdc *codec.Codec + supplyKeeper types.SupplyKeeper + hooks types.StakingHooks + paramstore params.Subspace + validatorCache map[string]cachedValidator + validatorCacheList *list.List + delayValidatorUpdates bool } // NewKeeper creates a new staking Keeper instance @@ -46,13 +47,14 @@ func NewKeeper( } return Keeper{ - storeKey: key, - cdc: cdc, - supplyKeeper: supplyKeeper, - paramstore: paramstore.WithKeyTable(ParamKeyTable()), - hooks: nil, - validatorCache: make(map[string]cachedValidator, aminoCacheSize), - validatorCacheList: list.New(), + storeKey: key, + cdc: cdc, + supplyKeeper: supplyKeeper, + paramstore: paramstore.WithKeyTable(ParamKeyTable()), + hooks: nil, + validatorCache: make(map[string]cachedValidator, aminoCacheSize), + validatorCacheList: list.New(), + delayValidatorUpdates: true, } } @@ -87,3 +89,8 @@ func (k Keeper) SetLastTotalPower(ctx sdk.Context, power sdk.Int) { b := k.cdc.MustMarshalBinaryLengthPrefixed(power) store.Set(types.LastTotalPowerKey, b) } + +// Allow turning off of validator update delays in tests +func (k *Keeper) SetDelayValidatorUpdates(delay bool) { + k.delayValidatorUpdates = delay +} diff --git a/x/staking/keeper/test_common.go b/x/staking/keeper/test_common.go index 0d730b48b3c..2a429710471 100644 --- a/x/staking/keeper/test_common.go +++ b/x/staking/keeper/test_common.go @@ -145,6 +145,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context keeper := NewKeeper(cdc, keyStaking, supplyKeeper, pk.Subspace(DefaultParamspace)) keeper.SetParams(ctx, types.DefaultParams()) + keeper.SetDelayValidatorUpdates(false) // set module accounts err = notBondedPool.SetCoins(totalSupply) diff --git a/x/staking/test_common.go b/x/staking/test_common.go index 84485e04168..c7b1073fc74 100644 --- a/x/staking/test_common.go +++ b/x/staking/test_common.go @@ -7,7 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/staking/types" - abci "github.com/tendermint/tendermint/abci/types" ) // nolint:deadcode,unused,varcheck @@ -56,10 +55,3 @@ func NewTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, amt sdk. amount := sdk.NewCoin(sdk.DefaultBondDenom, amt) return NewMsgDelegate(delAddr, valAddr, amount) } - -func testBlockEntropy() abci.BlockEntropy { - return abci.BlockEntropy{ - Round: 1, - AeonLength: 3, - } -}