From 76dd754f98e6497d98ee99e7c37341e1d30345c2 Mon Sep 17 00:00:00 2001
From: Manav Aggarwal <manavaggarwal1234@gmail.com>
Date: Wed, 16 Aug 2023 11:39:09 -0400
Subject: [PATCH] Fix store height update in manager (#1142)

<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Closes: #884

Applies the fix described in
https://github.com/rollkit/rollkit/issues/884#issuecomment-1520703683 -
update the store height before pushing the block to DA so that the store
height accessed in `trySyncNextBlock` is updated.

<!--
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue.
-->

## Checklist

<!--
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [x] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords
---
 block/manager.go | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/block/manager.go b/block/manager.go
index 3d2839c3d77..4678d7fedb6 100644
--- a/block/manager.go
+++ b/block/manager.go
@@ -313,7 +313,7 @@ func (m *Manager) SyncLoop(ctx context.Context, cancel context.CancelFunc) {
 			daHeight := blockEvent.daHeight
 			blockHash := block.Hash().String()
 			blockHeight := uint64(block.Height())
-			m.logger.Debug("block body retrieved from DALC",
+			m.logger.Debug("block body retrieved",
 				"height", blockHeight,
 				"daHeight", daHeight,
 				"hash", blockHash,
@@ -437,6 +437,8 @@ func (m *Manager) BlockStoreRetrieveLoop(ctx context.Context) {
 			daHeight := atomic.LoadUint64(&m.daHeight)
 			for _, block := range blocks {
 				m.blockInCh <- newBlockEvent{block, daHeight}
+				m.logger.Debug("block retrieved from p2p block sync", "blockHeight", block.Height(), "daHeight", daHeight)
+
 			}
 		}
 		lastBlockStoreHeight = blockStoreHeight
@@ -648,6 +650,11 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
+
+	blockHeight := uint64(block.Height())
+	// Update the stored height before submitting to the DA layer and committing to the DB
+	m.store.SetHeight(blockHeight)
+
 	blockHash := block.Hash().String()
 	m.blockCache.setSeen(blockHash)
 
@@ -669,8 +676,6 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 	m.pendingBlocks = append(m.pendingBlocks, block)
 	m.pendingBlocksMtx.Unlock()
 
-	blockHeight := uint64(block.SignedHeader.Header.Height())
-
 	// Commit the new state and block which writes to disk on the proxy app
 	_, _, err = m.executor.Commit(ctx, newState, block, responses)
 	if err != nil {
@@ -689,9 +694,6 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 		return err
 	}
 
-	// Only update the stored height after successfully submitting to DA layer and committing to the DB
-	m.store.SetHeight(blockHeight)
-
 	newState.DAHeight = atomic.LoadUint64(&m.daHeight)
 	// After this call m.lastState is the NEW state returned from ApplyBlock
 	// updateState also commits the DB tx