Skip to content

Commit

Permalink
Separate dkg and consensus validator updates for overlapping dkgs (co…
Browse files Browse the repository at this point in the history
…smos#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
  • Loading branch information
jinmannwong authored Aug 11, 2020
1 parent 473b82e commit 9f446eb
Show file tree
Hide file tree
Showing 19 changed files with 133 additions and 158 deletions.
8 changes: 4 additions & 4 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand All @@ -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(),
}
}
5 changes: 2 additions & 3 deletions x/distribution/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions x/distribution/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
15 changes: 4 additions & 11 deletions x/evidence/internal/keeper/infraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)),
Expand Down Expand Up @@ -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)
Expand All @@ -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)),
Expand All @@ -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,
}
}
2 changes: 1 addition & 1 deletion x/gov/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})

Expand Down
11 changes: 2 additions & 9 deletions x/gov/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
}
}
7 changes: 0 additions & 7 deletions x/gov/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
5 changes: 2 additions & 3 deletions x/slashing/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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
Expand Down
12 changes: 3 additions & 9 deletions x/slashing/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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,
}
}
16 changes: 7 additions & 9 deletions x/slashing/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down
21 changes: 9 additions & 12 deletions x/slashing/internal/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 1 addition & 7 deletions x/slashing/internal/keeper/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
}
8 changes: 4 additions & 4 deletions x/staking/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 9f446eb

Please sign in to comment.