Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(store): do not discard CAS fail errors #705

Merged
merged 4 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading