From e21a04ca573b574108f3a7ef0419bfc72ba26d4d Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Wed, 10 Mar 2021 10:56:23 -0700 Subject: [PATCH] fix: set previous block time correctly on block one (#868) * fix: set previous block time correctly on block one * fix failing tests --- x/validator-vesting/abci.go | 10 ++++------ x/validator-vesting/abci_test.go | 3 ++- x/validator-vesting/genesis.go | 5 ++++- x/validator-vesting/keeper/keeper.go | 6 +++--- x/validator-vesting/keeper/keeper_test.go | 9 ++++++--- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/x/validator-vesting/abci.go b/x/validator-vesting/abci.go index ac714946..1bebce04 100644 --- a/x/validator-vesting/abci.go +++ b/x/validator-vesting/abci.go @@ -2,9 +2,6 @@ package validatorvesting import ( "bytes" - "time" - - tmtime "github.com/tendermint/tendermint/types/time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -15,9 +12,10 @@ import ( // BeginBlocker updates the vote signing information for each validator vesting account, updates account when period changes, and updates the previousBlockTime value in the store. func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { - previousBlockTime := tmtime.Canonical(time.Unix(0, 0)) - if ctx.BlockHeight() > 1 { - previousBlockTime = k.GetPreviousBlockTime(ctx) + previousBlockTime, found := k.GetPreviousBlockTime(ctx) + if !found { + k.SetPreviousBlockTime(ctx, ctx.BlockTime()) + return } currentBlockTime := ctx.BlockTime() diff --git a/x/validator-vesting/abci_test.go b/x/validator-vesting/abci_test.go index 60d3a050..6c97ae78 100644 --- a/x/validator-vesting/abci_test.go +++ b/x/validator-vesting/abci_test.go @@ -69,7 +69,7 @@ func TestBeginBlockerZeroHeight(t *testing.T) { }}, }, } - + vvk.SetPreviousBlockTime(ctx, now) BeginBlocker(ctx, req, vvk) vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) @@ -147,6 +147,7 @@ func TestBeginBlockerSignedBlock(t *testing.T) { }, } + vvk.SetPreviousBlockTime(ctx, now) BeginBlocker(ctx, req, vvk) height++ blockTime = addHour(blockTime) diff --git a/x/validator-vesting/genesis.go b/x/validator-vesting/genesis.go index 00b6fd5f..fb083f44 100644 --- a/x/validator-vesting/genesis.go +++ b/x/validator-vesting/genesis.go @@ -22,6 +22,9 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, accountKeeper types.AccountKeep // ExportGenesis returns empty genesis state because auth exports all the genesis state we need. func ExportGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState { - prevBlockTime := keeper.GetPreviousBlockTime(ctx) + prevBlockTime, found := keeper.GetPreviousBlockTime(ctx) + if !found { + prevBlockTime = ctx.BlockTime() + } return NewGenesisState(prevBlockTime) } diff --git a/x/validator-vesting/keeper/keeper.go b/x/validator-vesting/keeper/keeper.go index 5469578a..3357eb7c 100644 --- a/x/validator-vesting/keeper/keeper.go +++ b/x/validator-vesting/keeper/keeper.go @@ -43,14 +43,14 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { } // GetPreviousBlockTime get the blocktime for the previous block -func (k Keeper) GetPreviousBlockTime(ctx sdk.Context) (blockTime time.Time) { +func (k Keeper) GetPreviousBlockTime(ctx sdk.Context) (blockTime time.Time, found bool) { store := ctx.KVStore(k.storeKey) b := store.Get(types.BlocktimeKey) if b == nil { - panic("Previous block time not set") + return time.Time{}, false } k.cdc.MustUnmarshalBinaryLengthPrefixed(b, &blockTime) - return blockTime + return blockTime, true } // SetPreviousBlockTime set the time of the previous block diff --git a/x/validator-vesting/keeper/keeper_test.go b/x/validator-vesting/keeper/keeper_test.go index 4f15b003..736c5864 100644 --- a/x/validator-vesting/keeper/keeper_test.go +++ b/x/validator-vesting/keeper/keeper_test.go @@ -67,19 +67,22 @@ func TestGetSetPreviousBlock(t *testing.T) { now := tmtime.Now() // require panic if the previous blocktime was never set - require.Panics(t, func() { keeper.GetPreviousBlockTime(ctx) }) + _, found := keeper.GetPreviousBlockTime(ctx) + require.False(t, found) // require that passing a valid time to SetPreviousBlockTime does not panic require.NotPanics(t, func() { keeper.SetPreviousBlockTime(ctx, now) }) // require that the value from GetPreviousBlockTime equals what was set - bpt := keeper.GetPreviousBlockTime(ctx) + bpt, found := keeper.GetPreviousBlockTime(ctx) + require.True(t, found) require.Equal(t, now, bpt) // require that the zero value is safe require.NotPanics(t, func() { keeper.SetPreviousBlockTime(ctx, tmtime.Canonical(time.Unix(0, 0))) }) - bpt = keeper.GetPreviousBlockTime(ctx) + bpt, found = keeper.GetPreviousBlockTime(ctx) + require.True(t, found) require.Equal(t, tmtime.Canonical(time.Unix(0, 0)), bpt) }