Skip to content

Commit

Permalink
fix(store): do not discard CAS fail errors (dymensionxyz#705)
Browse files Browse the repository at this point in the history
  • Loading branch information
danwt authored Apr 17, 2024
1 parent 849ba80 commit 3bcda30
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 24 deletions.
9 changes: 7 additions & 2 deletions block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package block

import (
"context"
"fmt"

"cosmossdk.io/errors"
"github.com/dymensionxyz/dymint/p2p"
Expand Down Expand Up @@ -115,7 +116,9 @@ func (m *Manager) applyBlock(ctx context.Context, block *types.Block, commit *ty
}
m.lastState = newState

m.store.SetHeight(block.Header.Height)
if ok := m.store.SetHeight(block.Header.Height); !ok {
return fmt.Errorf("store set height: %d", block.Header.Height)
}

return nil
}
Expand Down Expand Up @@ -189,7 +192,9 @@ func (m *Manager) UpdateStateFromApp() error {
if err != nil {
return errors.Wrap(err, "update state")
}
m.store.SetHeight(appHeight)
if ok := m.store.SetHeight(appHeight); !ok {
return fmt.Errorf("store set height: %d", appHeight)
}
return nil
}

Expand Down
12 changes: 7 additions & 5 deletions block/produce.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,19 @@ func (m *Manager) produceBlock(ctx context.Context, allowEmpty bool) error {
m.produceBlockMutex.Lock()
defer m.produceBlockMutex.Unlock()
var (
lastCommit *types.Commit
lastHeaderHash [32]byte
newHeight uint64
err error
lastCommit *types.Commit
lastHeaderHash [32]byte
newHeight uint64
err error
)

if m.lastState.IsGenesis() {
newHeight = uint64(m.lastState.InitialHeight)
lastCommit = &types.Commit{}
m.lastState.BaseHeight = newHeight
m.store.SetBase(newHeight)
if ok := m.store.SetBase(newHeight); !ok {
return fmt.Errorf("store set base: %d", newHeight)
}
} else {
height := m.store.Height()
newHeight = height + 1
Expand Down
4 changes: 3 additions & 1 deletion store/pruning.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ func (s *DefaultStore) PruneBlocks(heightInt int64) (uint64, error) {
if err != nil {
return fmt.Errorf("prune up to height %v: %w", base, err)
}
s.SetBase(base)
if ok := s.SetBase(base); !ok {
return fmt.Errorf("set base height: %v", base)
}
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion store/pruning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestStorePruning(t *testing.T) {

for _, block := range c.blocks {
_, err := bstore.SaveBlock(block, &types.Commit{}, nil)
bstore.SetHeight(block.Header.Height)
_ = bstore.SetHeight(block.Header.Height)
assert.NoError(err)
}

Expand Down
20 changes: 13 additions & 7 deletions store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ var (
type DefaultStore struct {
db KVStore

height uint64
baseHeight uint64
height uint64 // the highest block saved
baseHeight uint64 // the lowest block saved
}

var _ Store = &DefaultStore{}
Expand All @@ -47,11 +47,14 @@ func (s *DefaultStore) NewBatch() Batch {
}

// SetHeight sets the height saved in the Store if it is higher than the existing height
func (s *DefaultStore) SetHeight(height uint64) {
// returns OK if the value was updated successfully or did not need to be updated
func (s *DefaultStore) SetHeight(height uint64) bool {
ok := true
storeHeight := s.Height()
if height > storeHeight {
_ = atomic.CompareAndSwapUint64(&s.height, storeHeight, height)
ok = atomic.CompareAndSwapUint64(&s.height, storeHeight, height)
}
return ok
}

// Height returns height of the highest block saved in the Store.
Expand All @@ -64,12 +67,15 @@ func (s *DefaultStore) NextHeight() uint64 {
return s.Height() + 1
}

// SetBase sets the height saved in the Store of the earliest block
func (s *DefaultStore) SetBase(height uint64) {
// SetBase sets the base height if it is higher than the existing base height
// returns OK if the value was updated successfully or did not need to be updated
func (s *DefaultStore) SetBase(height uint64) bool {
ok := true
baseHeight := s.Base()
if height > baseHeight {
_ = atomic.CompareAndSwapUint64(&s.baseHeight, baseHeight, height)
ok = atomic.CompareAndSwapUint64(&s.baseHeight, baseHeight, height)
}
return ok
}

// Base returns height of the earliest block saved in the Store.
Expand Down
2 changes: 1 addition & 1 deletion store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestStoreHeight(t *testing.T) {

for _, block := range c.blocks {
_, err := bstore.SaveBlock(block, &types.Commit{}, nil)
bstore.SetHeight(block.Header.Height)
_ = bstore.SetHeight(block.Header.Height)
assert.NoError(err)
}

Expand Down
4 changes: 2 additions & 2 deletions store/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ type Store interface {
NextHeight() uint64

// SetHeight sets the height saved in the Store if it is higher than the existing height.
SetHeight(height uint64)
SetHeight(height uint64) bool

// Base returns height of the lowest block in store.
Base() uint64

// SetBase sets the height saved in the Store for the lowest block
SetBase(height uint64)
SetBase(height uint64) bool

// SaveBlock saves block along with its seen commit (which will be included in the next block).
SaveBlock(block *types.Block, commit *types.Commit, batch Batch) (Batch, error)
Expand Down
8 changes: 3 additions & 5 deletions testutil/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,18 @@ type MockStore struct {

// SetHeight sets the height of the mock store
// Don't set the height to mock failure in setting the height
func (m *MockStore) SetHeight(height uint64) {
// Fail the first time
func (m *MockStore) SetHeight(height uint64) bool {
if m.ShouldFailSetHeight {
return
return false
}
m.height = height
return true
}

// Height returns the height of the mock store
func (m *MockStore) Height() uint64 {
return m.height
}

// Height returns the height of the mock store
func (m *MockStore) NextHeight() uint64 {
return m.height + 1
}
Expand Down

0 comments on commit 3bcda30

Please sign in to comment.