diff --git a/block/block.go b/block/block.go index 815e5c55e..567ff3dbf 100644 --- a/block/block.go +++ b/block/block.go @@ -86,7 +86,7 @@ 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. - _ = m.Executor.UpdateStateAfterCommit(m.State, responses, appHash, block.Header.Height, validators) + 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) diff --git a/block/executor_test.go b/block/executor_test.go index eaf8f531d..e4eca2f55 100644 --- a/block/executor_test.go +++ b/block/executor_test.go @@ -183,7 +183,7 @@ func TestApplyBlock(t *testing.T) { require.NotNil(resp) appHash, _, err := executor.Commit(state, block, resp) require.NoError(err) - _ = 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) @@ -236,7 +236,7 @@ func TestApplyBlock(t *testing.T) { require.NoError(err) _, _, err = executor.Commit(state, block, resp) require.NoError(err) - _ = 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. diff --git a/block/manager_test.go b/block/manager_test.go index 6bc541d10..348850165 100644 --- a/block/manager_test.go +++ b/block/manager_test.go @@ -240,7 +240,7 @@ func TestProduceBlockFailAfterCommit(t *testing.T) { cases := []struct { name string - shoudFailSaveState bool + shoudFailOnSaveState bool LastAppBlockHeight int64 AppCommitHash [32]byte LastAppCommitHash [32]byte @@ -248,47 +248,37 @@ func TestProduceBlockFailAfterCommit(t *testing.T) { expectedStateAppHash [32]byte }{ { - name: "ProduceFirstBlockSuccessfully", - shoudFailSaveState: false, - AppCommitHash: [32]byte{1}, - // LastAppCommitHash: [32]byte{0}, - // LastAppBlockHeight: 0, + name: "ProduceFirstBlockSuccessfully", + shoudFailOnSaveState: false, + AppCommitHash: [32]byte{1}, expectedStoreHeight: 1, expectedStateAppHash: [32]byte{1}, }, { - name: "ProduceSecondBlockFailOnUpdateState", - shoudFailSaveState: true, - AppCommitHash: [32]byte{2}, - // LastAppCommitHash: [32]byte{}, - // LastAppBlockHeight: 0, - // state not changed on failed save state - expectedStoreHeight: 1, + name: "ProduceSecondBlockFailOnUpdateState", + shoudFailOnSaveState: true, + AppCommitHash: [32]byte{2}, + expectedStoreHeight: 1, // height not changed on failed save state expectedStateAppHash: [32]byte{1}, }, { - name: "ProduceSecondBlockSuccessfullyFromApp", - shoudFailSaveState: false, - // AppCommitHash: [32]byte{}, - // expected return from app - LastAppCommitHash: [32]byte{2}, + name: "ProduceSecondBlockSuccessfullyFromApp", + shoudFailOnSaveState: false, + LastAppCommitHash: [32]byte{2}, // loading state from app LastAppBlockHeight: 2, expectedStoreHeight: 2, expectedStateAppHash: [32]byte{2}, }, { - name: "ProduceThirdBlockFailOnUpdateStoreHeight", - shoudFailSaveState: true, - AppCommitHash: [32]byte{3}, - // LastAppCommitHash: [32]byte{2}, - // LastAppBlockHeight: 2, - expectedStoreHeight: 2, + name: "ProduceThirdBlockFailOnUpdateStoreHeight", + shoudFailOnSaveState: true, + AppCommitHash: [32]byte{3}, + expectedStoreHeight: 2, // height not changed on failed save state expectedStateAppHash: [32]byte{2}, }, { name: "ProduceThirdBlockSuccessfully", - shoudFailSaveState: false, - AppCommitHash: [32]byte{}, + shoudFailOnSaveState: false, LastAppCommitHash: [32]byte{3}, LastAppBlockHeight: 3, expectedStoreHeight: 3, @@ -302,7 +292,7 @@ func TestProduceBlockFailAfterCommit(t *testing.T) { LastBlockHeight: tc.LastAppBlockHeight, LastBlockAppHash: tc.LastAppCommitHash[:], }) - mockStore.ShoudFailSaveState = tc.shoudFailSaveState + mockStore.ShoudFailSaveState = tc.shoudFailOnSaveState _, _, _ = manager.ProduceAndGossipBlock(context.Background(), true) storeState, err := manager.Store.LoadState() assert.NoError(err) diff --git a/block/retriever.go b/block/retriever.go index 420ca41fd..240d1a902 100644 --- a/block/retriever.go +++ b/block/retriever.go @@ -76,7 +76,7 @@ func (m *Manager) queryStateIndex() (uint64, error) { var stateIndex uint64 return stateIndex, retry.Do( func() error { - res, err := m.SLClient.GetHeightState(m.State.Height() + 1) + res, err := m.SLClient.GetHeightState(m.State.NextHeight()) if err != nil { m.logger.Debug("sl client get height state", "error", err) return err diff --git a/block/state.go b/block/state.go index ed9209c14..1207df93a 100644 --- a/block/state.go +++ b/block/state.go @@ -49,7 +49,7 @@ func (m *Manager) UpdateStateFromApp() error { } // update the state with the hash, last store height and last validators. - _ = m.Executor.UpdateStateAfterCommit(m.State, resp, proxyAppInfo.LastBlockAppHash, appHeight, vals) + 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") @@ -109,29 +109,10 @@ func (e *Executor) NextValSetFromResponses(state *types.State, resp *tmstate.ABC // Dymint ignores any setValidator responses from the app, as it is manages the validator set based on the settlement consensus // TODO: this will be changed when supporting multiple sequencers from the hub return state.NextValidators.Copy(), nil - - /* - nValSet := state.NextValidators.Copy() - lastHeightValSetChanged := state.LastHeightValidatorsChanged - // Dymint can work without validators - if len(nValSet.Validators) > 0 { - if len(validatorUpdates) > 0 { - err := nValSet.UpdateWithChangeSet(validatorUpdates) - if err != nil { - return err - } - // Change results from this height but only applies to the next next height. - lastHeightValSetChanged = int64(block.Header.Height + 1 + 1) - } - - // TODO(tzdybal): right now, it's for backward compatibility, may need to change this - nValSet.IncrementProposerPriority(1) - } - */ } // 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) { copy(s.AppHash[:], appHash[:]) copy(s.LastResultsHash[:], tmtypes.NewResults(resp.DeliverTxs).Hash()) @@ -141,5 +122,4 @@ func (e *Executor) UpdateStateAfterCommit(s *types.State, resp *tmstate.ABCIResp s.NextValidators = valSet.Copy() s.SetHeight(height) - return s } diff --git a/store/pruning.go b/store/pruning.go index 78d2ce0b4..098451783 100644 --- a/store/pruning.go +++ b/store/pruning.go @@ -10,7 +10,7 @@ func (s *DefaultStore) PruneBlocks(from, to uint64) (uint64, error) { return 0, fmt.Errorf("from height must be greater than 0") } - if to < from { + if to <= from { return 0, fmt.Errorf("cannot prune to height %v, it is lower than base height %v", to, from) } diff --git a/store/pruning_test.go b/store/pruning_test.go index fc3fea85d..850d4a9ad 100644 --- a/store/pruning_test.go +++ b/store/pruning_test.go @@ -19,7 +19,6 @@ func TestStorePruning(t *testing.T) { to uint64 shouldError bool }{ - // todo :check exclusion of pruning height {"blocks with pruning", []*types.Block{ testutil.GetRandomBlock(1, 0), @@ -44,8 +43,12 @@ func TestStorePruning(t *testing.T) { testutil.GetRandomBlock(2, 0), testutil.GetRandomBlock(3, 0), }, 0, 1, true}, + {"pruning same height", []*types.Block{ + testutil.GetRandomBlock(1, 0), + testutil.GetRandomBlock(2, 0), + testutil.GetRandomBlock(3, 0), + }, 3, 3, true}, } - for _, c := range cases { t.Run(c.name, func(t *testing.T) { assert := assert.New(t) @@ -76,7 +79,7 @@ func TestStorePruning(t *testing.T) { // Validate only blocks in the range are pruned for k := range savedHeights { - if k >= c.from && k < c.to { + if k >= c.from && k < c.to { //k < c.to is the exclusion test _, err := bstore.LoadBlock(k) assert.Error(err, "Block at height %d should be pruned", k) diff --git a/types/state.go b/types/state.go index d113dd092..c74e2871e 100644 --- a/types/state.go +++ b/types/state.go @@ -103,14 +103,3 @@ func (s *State) NextHeight() uint64 { } return s.Height() + 1 } - -// 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 *State) SetBase(height uint64) { - s.BaseHeight = height -} - -// Base returns height of the earliest block saved in the Store. -func (s *State) Base() uint64 { - return s.BaseHeight -}