Skip to content

Commit

Permalink
Fix store height update in manager (cosmos#1142)
Browse files Browse the repository at this point in the history
<!--
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: cosmos#884 

Applies the fix described in
rollkit/rollkit#884 (comment) -
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
  • Loading branch information
Manav-Aggarwal committed Aug 16, 2023
1 parent 1acf092 commit 76dd754
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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 {
Expand All @@ -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
Expand Down

0 comments on commit 76dd754

Please sign in to comment.