From bca99b191dd658095228d351ac343fc21c6b23c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Aasted=20S=C3=B8rensen?= Date: Wed, 22 Sep 2021 17:26:09 +0200 Subject: [PATCH 1/2] Ensure that genesis-time is not used for jailing and slashing --- x/slashing/keeper/abci.go | 8 +++- x/slashing/keeper/abci_test.go | 82 ++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/x/slashing/keeper/abci.go b/x/slashing/keeper/abci.go index f3dee249..cbddf7f2 100644 --- a/x/slashing/keeper/abci.go +++ b/x/slashing/keeper/abci.go @@ -25,8 +25,12 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k Keeper) { blockTimes := k.getBlockTimes() blockTimes = append(blockTimes, ctx.BlockTime()) slashable := false - slashable, blockTimes = truncateByWindow(ctx.BlockTime(), blockTimes, signedBlocksWindow) - k.setBlockTimes(batch, blockTimes) + + if ctx.BlockHeight() != 1 { + // Ignore first blocktime, which comes from the genesis file and may not match actual time + slashable, blockTimes = truncateByWindow(ctx.BlockTime(), blockTimes, signedBlocksWindow) + k.setBlockTimes(batch, blockTimes) + } // Iterate over all the validators which *should* have signed this block // store whether or not they have actually signed it and slash/unbond any diff --git a/x/slashing/keeper/abci_test.go b/x/slashing/keeper/abci_test.go index 032e2ffb..bed44577 100644 --- a/x/slashing/keeper/abci_test.go +++ b/x/slashing/keeper/abci_test.go @@ -196,6 +196,88 @@ func TestBeginBlocker(t *testing.T) { } +func TestOldGenesisTime(t *testing.T) { + // The launch of emoney-3 revealed that the first block contains the genesis time from genesis.json. In the case of emoney-3, this was inherited directly from + // emoney-2 and was thus some time in the past, leading to immediate jailing and slashing of validators that were not ready. + // Block 1 should be treated differently to avoid this in future upgrades. + genesisTime, _ := time.Parse(time.RFC3339, "2020-11-04T13:00:00Z") // Genesis time from emoney-2 and emoney-3 + + ctx, keeper, _, _, stakingKeeper, database := createTestComponents(t) + + var power1, power2 int64 = 70, 30 + addr1, pk1 := addrs[2], pks[2] + addr2, pk2 := addrs[1], pks[1] + + // bond the validators + _, err := staking.NewHandler(stakingKeeper)(ctx, NewTestMsgCreateValidator(addr1, pk1, sdk.TokensFromConsensusPower(power1))) + require.NoError(t, err) + _, err = staking.NewHandler(stakingKeeper)(ctx, NewTestMsgCreateValidator(addr2, pk2, sdk.TokensFromConsensusPower(power2))) + require.NoError(t, err) + + val1 := abci.Validator{pk1.Address(), power1} + val2 := abci.Validator{pk2.Address(), power2} + + // Val2 fails to sign + req := abci.RequestBeginBlock{ + LastCommitInfo: abci.LastCommitInfo{ + Votes: []abci.VoteInfo{ + { + Validator: val1, + SignedLastBlock: true, + }, + { + Validator: val2, + SignedLastBlock: false, + }, + }, + }, + } + + // Start processing blocks + now := time.Now() + ctx = ctx.WithBlockHeight(1).WithBlockTime(genesisTime) + + // block 1 + { + batch := database.NewBatch() + ctx = apptypes.WithCurrentBatch(ctx, batch) + BeginBlocker(ctx, req, keeper) + staking.EndBlocker(ctx, stakingKeeper) + batch.Write() + } + + ctx = ctx.WithBlockHeight(2).WithBlockTime(now) + // block 2 + { + batch := database.NewBatch() + ctx = apptypes.WithCurrentBatch(ctx, batch) + BeginBlocker(ctx, req, keeper) + staking.EndBlocker(ctx, stakingKeeper) + batch.Write() + } + + // Validator 2 must remain bonded. + validator, found := stakingKeeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk2)) + require.True(t, found) + require.Equal(t, stakingtypes.Bonded, validator.GetStatus()) + + // create a third block 25 hours later and ensure jailing + ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(25 * time.Hour)) + // block 3 + { + batch := database.NewBatch() + ctx = apptypes.WithCurrentBatch(ctx, batch) + BeginBlocker(ctx, req, keeper) + staking.EndBlocker(ctx, stakingKeeper) + batch.Write() + } + + // Validator 2 must be unbonding. + validator, found = stakingKeeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk2)) + require.True(t, found) + require.Equal(t, stakingtypes.Unbonding, validator.GetStatus()) +} + func createTestComponents(t *testing.T) (sdk.Context, Keeper, banktypes.AccountKeeper, bankkeeper.Keeper, stakingkeeper.Keeper, *dbm.MemDB) { t.Helper() encConfig := MakeTestEncodingConfig() From c5b1b7449dd2a5cbe4946cc2ff213ccbaf106bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Aasted=20S=C3=B8rensen?= Date: Thu, 23 Sep 2021 15:37:14 +0200 Subject: [PATCH 2/2] Code review --- x/slashing/keeper/abci_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/slashing/keeper/abci_test.go b/x/slashing/keeper/abci_test.go index bed44577..0f7271f1 100644 --- a/x/slashing/keeper/abci_test.go +++ b/x/slashing/keeper/abci_test.go @@ -262,7 +262,7 @@ func TestOldGenesisTime(t *testing.T) { require.Equal(t, stakingtypes.Bonded, validator.GetStatus()) // create a third block 25 hours later and ensure jailing - ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(25 * time.Hour)) + ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(61 * time.Minute)) // block 3 { batch := database.NewBatch() @@ -275,7 +275,7 @@ func TestOldGenesisTime(t *testing.T) { // Validator 2 must be unbonding. validator, found = stakingKeeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk2)) require.True(t, found) - require.Equal(t, stakingtypes.Unbonding, validator.GetStatus()) + require.Equal(t, stakingtypes.Unbonding.String(), validator.GetStatus().String()) } func createTestComponents(t *testing.T) (sdk.Context, Keeper, banktypes.AccountKeeper, bankkeeper.Keeper, stakingkeeper.Keeper, *dbm.MemDB) {