fixed PR comments

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)

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.

block/manager_test.go

@@ -240,55 +240,45 @@ func TestProduceBlockFailAfterCommit(t *testing.T) {
 
 	cases := []struct {
 		name                 string
-		shoudFailSaveState   bool
+		shoudFailOnSaveState bool
 		LastAppBlockHeight   int64
 		AppCommitHash        [32]byte
 		LastAppCommitHash    [32]byte
 		expectedStoreHeight  uint64
 		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)

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

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
 }

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)
 	}

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)
 

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
-}