Skip to content

Commit

Permalink
fixed LastBlockHeight to be atomic
Browse files Browse the repository at this point in the history
  • Loading branch information
mtsitrin committed May 13, 2024
1 parent efcf193 commit 3992f2a
Show file tree
Hide file tree
Showing 13 changed files with 49 additions and 52 deletions.
7 changes: 3 additions & 4 deletions block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (m *Manager) applyBlock(block *types.Block, commit *types.Commit, blockMeta
return fmt.Errorf("save block responses: %w", err)
}

// Updates the state with validator changes and consensus params changes from the app
// Get the validator changes from the app
validators, err := m.Executor.NextValSetFromResponses(m.State, responses, block)
if err != nil {
return fmt.Errorf("update state from responses: %w", err)
Expand Down Expand Up @@ -86,12 +86,11 @@ func (m *Manager) applyBlock(block *types.Block, commit *types.Commit, blockMeta

// Update the state with the new app hash, last validators and store height from the commit.
// Every one of those, if happens before commit, prevents us from re-executing the block in case failed during commit.
newState := m.Executor.UpdateStateAfterCommit(m.State, responses, appHash, block.Header.Height, validators)
_, err = m.Store.SaveState(newState, nil)
_ = m.Executor.UpdateStateAfterCommit(&m.State, responses, appHash, block.Header.Height, validators)
_, err = m.Store.SaveState(m.State, nil)
if err != nil {
return fmt.Errorf("update state: %w", err)
}
m.State = newState

// Prune old heights, if requested by ABCI app.
if retainHeight > 0 {
Expand Down
7 changes: 3 additions & 4 deletions block/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestApplyBlock(t *testing.T) {
Validators: tmtypes.NewValidatorSet(nil),
}
state.InitialHeight = 1
state.LastBlockHeight = 0
state.LastBlockHeight.Store(0)
maxBytes := uint64(100)
state.ConsensusParams.Block.MaxBytes = int64(maxBytes)
state.ConsensusParams.Block.MaxGas = 100000
Expand Down Expand Up @@ -183,7 +183,7 @@ func TestApplyBlock(t *testing.T) {
require.NotNil(resp)
appHash, _, err := executor.Commit(state, block, resp)
require.NoError(err)
state = executor.UpdateStateAfterCommit(state, resp, appHash, block.Header.Height, state.Validators)
_ = executor.UpdateStateAfterCommit(&state, resp, appHash, block.Header.Height, state.Validators)
assert.Equal(uint64(1), state.Height())
assert.Equal(mockAppHash, state.AppHash)

Expand Down Expand Up @@ -236,8 +236,7 @@ func TestApplyBlock(t *testing.T) {
require.NoError(err)
_, _, err = executor.Commit(state, block, resp)
require.NoError(err)
state = executor.UpdateStateAfterCommit(state, resp, appHash, block.Header.Height, vals)

_ = executor.UpdateStateAfterCommit(&state, resp, appHash, block.Header.Height, vals)
assert.Equal(uint64(2), state.Height())

// wait for at least 4 Tx events, for up to 3 second.
Expand Down
5 changes: 3 additions & 2 deletions block/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestInitialState(t *testing.T) {
store: fullStore,
genesis: genesis,
expectedInitialHeight: sampleState.InitialHeight,
expectedLastBlockHeight: sampleState.LastBlockHeight,
expectedLastBlockHeight: sampleState.LastBlockHeight.Load(),
expectedChainID: sampleState.ChainID,
},
}
Expand All @@ -100,7 +100,7 @@ func TestInitialState(t *testing.T) {
assert.NotNil(agg)
assert.Equal(c.expectedChainID, agg.State.ChainID)
assert.Equal(c.expectedInitialHeight, agg.State.InitialHeight)
assert.Equal(c.expectedLastBlockHeight, agg.State.LastBlockHeight)
assert.Equal(c.expectedLastBlockHeight, agg.State.LastBlockHeight.Load())
})
}
}
Expand Down Expand Up @@ -306,6 +306,7 @@ func TestProduceBlockFailAfterCommit(t *testing.T) {
_, _, _ = manager.ProduceAndGossipBlock(context.Background(), true)
storeState, err := manager.Store.LoadState()
assert.NoError(err)
manager.State = storeState
assert.Equal(tc.expectedStoreHeight, storeState.Height(), tc.name)
assert.Equal(tc.expectedStateAppHash, storeState.AppHash, tc.name)

Expand Down
6 changes: 2 additions & 4 deletions block/pruning.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ func (m *Manager) pruneBlocks(retainHeight uint64) (uint64, error) {

// TODO: prune state/indexer and state/txindexer??

newState := m.State
newState.BaseHeight = retainHeight
_, err = m.Store.SaveState(newState, nil)
m.State.BaseHeight = retainHeight
_, err = m.Store.SaveState(m.State, nil)
if err != nil {
return 0, fmt.Errorf("final update state: %w", err)
}
m.State = newState

m.logger.Info("pruned blocks", "pruned", pruned, "retain_height", retainHeight)
return pruned, nil
Expand Down
7 changes: 3 additions & 4 deletions block/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,11 @@ func (m *Manager) UpdateStateFromApp() error {
}

// update the state with the hash, last store height and last validators.
state := m.Executor.UpdateStateAfterCommit(m.State, resp, proxyAppInfo.LastBlockAppHash, appHeight, vals)
_, err = m.Store.SaveState(state, nil)
_ = m.Executor.UpdateStateAfterCommit(&m.State, resp, proxyAppInfo.LastBlockAppHash, appHeight, vals)
_, err = m.Store.SaveState(m.State, nil)
if err != nil {
return errorsmod.Wrap(err, "update state")
}
m.State = state
return nil
}

Expand Down Expand Up @@ -132,7 +131,7 @@ func (e *Executor) NextValSetFromResponses(state types.State, resp *tmstate.ABCI
}

// Update state from Commit response
func (e *Executor) UpdateStateAfterCommit(s types.State, resp *tmstate.ABCIResponses, appHash []byte, height uint64, valSet *tmtypes.ValidatorSet) types.State {
func (e *Executor) UpdateStateAfterCommit(s *types.State, resp *tmstate.ABCIResponses, appHash []byte, height uint64, valSet *tmtypes.ValidatorSet) *types.State {
copy(s.AppHash[:], appHash[:])
copy(s.LastResultsHash[:], tmtypes.NewResults(resp.DeliverTxs).Hash())

Expand Down
1 change: 0 additions & 1 deletion node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ func (n *Node) startPrometheusServer() error {
return nil
}

// FIXME: read from block manager
func (n *Node) GetBlockManagerHeight() uint64 {
return n.BlockManager.State.Height()
}
11 changes: 0 additions & 11 deletions store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/binary"
"errors"
"fmt"
"sync/atomic"

tmstate "github.com/tendermint/tendermint/proto/tendermint/state"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
Expand Down Expand Up @@ -45,16 +44,6 @@ func (s *DefaultStore) NewBatch() Batch {
return s.db.NewBatch()
}

// SetHeight sets the height of the store
func (s *DefaultStore) SetHeight(height uint64) {
atomic.StoreUint64(&s.height, height)
}

// Height returns height of the highest block saved in the Store.
func (s *DefaultStore) Height() uint64 {
return atomic.LoadUint64(&s.height)
}

// SaveBlock adds block to the store along with corresponding commit.
// Stored height is updated if block height is greater than stored value.
// In case a batch is provided, the block and commit are added to the batch and not saved.
Expand Down
13 changes: 7 additions & 6 deletions store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,19 @@ func TestLoadState(t *testing.T) {
kv := store.NewDefaultInMemoryKVStore()
s1 := store.New(kv)
expectedHeight := uint64(10)
_, err := s1.SaveState(types.State{
LastBlockHeight: expectedHeight,
NextValidators: validatorSet,
Validators: validatorSet,
}, nil)
s := types.State{
NextValidators: validatorSet,
Validators: validatorSet,
}
s.LastBlockHeight.Store(expectedHeight)
_, err := s1.SaveState(s, nil)
assert.NoError(err)

s2 := store.New(kv)
state, err := s2.LoadState()
assert.NoError(err)

assert.Equal(expectedHeight, state.LastBlockHeight)
assert.Equal(expectedHeight, state.LastBlockHeight.Load())
}

func TestBlockResponses(t *testing.T) {
Expand Down
13 changes: 10 additions & 3 deletions test/loadtime/cmd/report/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ var mainPrefix = [1]byte{0}
// BlockStore is a thin wrapper around the DefaultStore which will be used for inspecting the blocks
type BlockStore struct {
*store.DefaultStore
base uint64
base uint64
height uint64
}

// Height implements report.BlockStore.
func (b *BlockStore) Height() uint64 {
return b.height
}

// Base will be used to get the block height of the first block we want to generate the report for
Expand All @@ -40,13 +46,14 @@ func getStore(directory string) *store.PrefixKV {

func newBlockStore(kvstore store.KVStore, baseHeight uint64) *BlockStore {
store := store.New(kvstore).(*store.DefaultStore)
_, err := store.LoadState()
state, err := store.LoadState()
if err != nil {
log.Fatalf("loading state %s", err)
}
return &BlockStore{
DefaultStore: store,
base: baseHeight,
base: state.BaseHeight,
height: state.LastBlockHeight.Load(),
}
}

Expand Down
9 changes: 5 additions & 4 deletions testutil/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func GenerateRandomValidatorSet() *tmtypes.ValidatorSet {

// GenerateState generates an initial state for testing.
func GenerateState(initialHeight int64, lastBlockHeight int64) types.State {
return types.State{
s := types.State{
ChainID: "test-chain",
InitialHeight: uint64(initialHeight),
AppHash: [32]byte{},
Expand All @@ -205,10 +205,11 @@ func GenerateState(initialHeight int64, lastBlockHeight int64) types.State {
App: AppVersion,
},
},
LastBlockHeight: uint64(lastBlockHeight),
Validators: GenerateRandomValidatorSet(),
NextValidators: GenerateRandomValidatorSet(),
Validators: GenerateRandomValidatorSet(),
NextValidators: GenerateRandomValidatorSet(),
}
s.LastBlockHeight.Store(uint64(lastBlockHeight))
return s
}

// GenerateGenesis generates a genesis for testing.
Expand Down
4 changes: 2 additions & 2 deletions types/serialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func (s *State) ToProto() (*pb.State, error) {
Version: &s.Version,
ChainId: s.ChainID,
InitialHeight: int64(s.InitialHeight),
LastBlockHeight: int64(s.LastBlockHeight),
LastBlockHeight: int64(s.LastBlockHeight.Load()),
NextValidators: nextValidators,
Validators: validators,
BaseHeight: s.BaseHeight,
Expand All @@ -274,7 +274,7 @@ func (s *State) FromProto(other *pb.State) error {
s.Version = *other.Version
s.ChainID = other.ChainId
s.InitialHeight = uint64(other.InitialHeight)
s.LastBlockHeight = uint64(other.LastBlockHeight)
s.LastBlockHeight.Store(uint64(other.LastBlockHeight))
s.BaseHeight = other.BaseHeight

s.NextValidators, err = types.ValidatorSetFromProto(other.NextValidators)
Expand Down
6 changes: 5 additions & 1 deletion types/serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ func TestStateRoundTrip(t *testing.T) {
},
ChainID: "testchain",
InitialHeight: 987,
LastBlockHeight: 987654321,
NextValidators: valSet,
Validators: valSet,
LastHeightValidatorsChanged: 8272,
Expand Down Expand Up @@ -153,6 +152,11 @@ func TestStateRoundTrip(t *testing.T) {
t.Run(c.name, func(t *testing.T) {
require := require.New(t)
assert := assert.New(t)

if c.state.InitialHeight != 0 {
c.state.LastBlockHeight.Store(986321)
}

pState, err := c.state.ToProto()
require.NoError(err)
require.NotNil(pState)
Expand Down
12 changes: 6 additions & 6 deletions types/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"fmt"
"sync/atomic"

// TODO(tzdybal): copy to local project?
tmstate "github.com/tendermint/tendermint/proto/tendermint/state"
Expand All @@ -20,8 +21,7 @@ type State struct {
InitialHeight uint64 // should be 1, not 0, when starting from height 1

// LastBlockHeight=0 at genesis (ie. block(H=0) does not exist)
//TODO: should be atomic as can be queried by the RPC
LastBlockHeight uint64
LastBlockHeight atomic.Uint64

// BaseHeight is the height of the first block we have in store after pruning.
BaseHeight uint64
Expand Down Expand Up @@ -66,8 +66,7 @@ func NewStateFromGenesis(genDoc *types.GenesisDoc) (State, error) {
ChainID: genDoc.ChainID,
InitialHeight: uint64(genDoc.InitialHeight),

LastBlockHeight: 0,
BaseHeight: uint64(genDoc.InitialHeight),
BaseHeight: uint64(genDoc.InitialHeight),

NextValidators: types.NewValidatorSet(nil),
Validators: types.NewValidatorSet(nil),
Expand All @@ -76,6 +75,7 @@ func NewStateFromGenesis(genDoc *types.GenesisDoc) (State, error) {
ConsensusParams: *genDoc.ConsensusParams,
LastHeightConsensusParamsChanged: genDoc.InitialHeight,
}
s.LastBlockHeight.Store(0)
copy(s.AppHash[:], genDoc.AppHash)

return s, nil
Expand All @@ -88,12 +88,12 @@ func (s *State) IsGenesis() bool {
// SetHeight sets the height saved in the Store if it is higher than the existing height
// returns OK if the value was updated successfully or did not need to be updated
func (s *State) SetHeight(height uint64) {
s.LastBlockHeight = height
s.LastBlockHeight.Store(height)
}

// Height returns height of the highest block saved in the Store.
func (s *State) Height() uint64 {
return s.LastBlockHeight
return s.LastBlockHeight.Load()
}

// NextHeight returns the next height that expected to be stored in store.
Expand Down

0 comments on commit 3992f2a

Please sign in to comment.