Skip to content
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## v1.0.0-rc.3

### Added

- Add DA Hints for P2P transactions. This allows a catching up node to be on sync with both DA and P2P. ([#2891](https://github.com/evstack/ev-node/pull/2891))
Expand All @@ -18,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve `cache.NumPendingData` to not return empty data. Automatically bumps `LastSubmittedHeight` to reflect that. ([#3046](https://github.com/evstack/ev-node/pull/3046))
- **BREAKING** Make pending events cache and tx cache fully ephemeral. Those will be re-fetched on restart. DA Inclusion cache persists until cleared up after DA inclusion has been processed. Persist accross restart using store metadata. ([#3047](https://github.com/evstack/ev-node/pull/3047))
- Replace LRU cache by standard mem cache with manual eviction in `store_adapter`. When P2P blocks were fetched too fast, they would be evicted before being executed [#3051](https://github.com/evstack/ev-node/pull/3051)
- Fix replay logic leading to app hashes by verifying against the wrong block [#3053](https://github.com/evstack/ev-node/pull/3053).

## v1.0.0-rc.2

Expand Down
66 changes: 36 additions & 30 deletions block/internal/common/replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,27 +150,21 @@ func (s *Replayer) replayBlock(ctx context.Context, height uint64) error {
// Get the previous state
var prevState types.State
if height == s.genesis.InitialHeight {
// For the first block, use genesis state
// For the first block, use genesis state.
prevState = types.State{
ChainID: s.genesis.ChainID,
InitialHeight: s.genesis.InitialHeight,
LastBlockHeight: s.genesis.InitialHeight - 1,
LastBlockTime: s.genesis.StartTime,
AppHash: header.AppHash, // This will be updated by InitChain
AppHash: header.AppHash, // Genesis app hash (input to first block execution)
}
} else {
// Get previous state from store
// GetStateAtHeight(height-1) returns the state AFTER block height-1 was executed,
// which contains the correct AppHash to use as input for executing block at 'height'.
prevState, err = s.store.GetStateAtHeight(ctx, height-1)
if err != nil {
return fmt.Errorf("failed to get previous state: %w", err)
}
// We need the state at height-1, so load that block's app hash
prevHeader, _, err := s.store.GetBlockData(ctx, height-1)
if err != nil {
return fmt.Errorf("failed to get previous block header: %w", err)
}
prevState.AppHash = prevHeader.AppHash
prevState.LastBlockHeight = height - 1
}

// Prepare transactions
Expand All @@ -190,27 +184,39 @@ func (s *Replayer) replayBlock(ctx context.Context, height uint64) error {
return fmt.Errorf("failed to execute transactions: %w", err)
}

// DEBUG: Log comparison of expected vs actual app hash
s.logger.Debug().
Uint64("height", height).
Str("expected_app_hash", hex.EncodeToString(header.AppHash)).
Str("actual_app_hash", hex.EncodeToString(newAppHash)).
Bool("hashes_match", bytes.Equal(newAppHash, header.AppHash)).
Msg("replayBlock: ExecuteTxs completed")

// Verify the app hash matches
if !bytes.Equal(newAppHash, header.AppHash) {
err := fmt.Errorf("app hash mismatch: expected %s got %s",
hex.EncodeToString(header.AppHash),
hex.EncodeToString(newAppHash),
)
s.logger.Error().
Str("expected", hex.EncodeToString(header.AppHash)).
Str("got", hex.EncodeToString(newAppHash)).
// The result of ExecuteTxs (newAppHash) should match the stored state at this height.
// Note: header.AppHash is the PREVIOUS state's app hash (input), not the expected output.
// The expected output is stored in state[height].AppHash or equivalently in header[height+1].AppHash.

// For verification, we need to get the expected app hash from the stored state at this height.
// If this state doesn't exist (which would be unusual since we fetched the block), we skip verification.
expectedState, err := s.store.GetStateAtHeight(ctx, height)
if err == nil {
// State exists, verify the app hash matches
if !bytes.Equal(newAppHash, expectedState.AppHash) {
err := fmt.Errorf("app hash mismatch at height %d: expected %s got %s",
height,
hex.EncodeToString(expectedState.AppHash),
hex.EncodeToString(newAppHash),
)
s.logger.Error().
Str("expected", hex.EncodeToString(expectedState.AppHash)).
Str("got", hex.EncodeToString(newAppHash)).
Uint64("height", height).
Err(err).
Msg("app hash mismatch during replay")
return err
}
s.logger.Debug().
Uint64("height", height).
Str("app_hash", hex.EncodeToString(newAppHash)).
Msg("replayBlock: app hash verified against stored state")
} else {
// State doesn't exist yet, we trust the execution result since we're replaying validated blocks.
s.logger.Debug().
Uint64("height", height).
Err(err).
Msg("app hash mismatch during replay")
return err
Str("app_hash", hex.EncodeToString(newAppHash)).
Msg("replayBlock: ExecuteTxs completed (no stored state to verify against)")
}
Comment on lines +194 to 220
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This logic handles errors from s.store.GetStateAtHeight by assuming any error means the state was not found, and then proceeds without verification. This is risky because it could mask other critical errors from the store, such as data corruption, disk I/O issues, or unmarshalling problems. In such cases, the replayer would silently proceed and potentially persist an incorrect state.

It would be safer to explicitly check for a 'not found' error and treat all other errors as fatal for the replay of this block.

A more robust implementation would be to check for the specific 'not found' error. For example:

expectedState, err := s.store.GetStateAtHeight(ctx, height)
if err != nil {
    // It's important to distinguish 'not found' from other errors.
    // This might require exporting an error from the store package or using errors.Is with the underlying datastore error.
    if !isNotFoundError(err) { // isNotFoundError is a placeholder for the actual check
        return fmt.Errorf("failed to get expected state for verification at height %d: %w", height, err)
    }

    // State not found, proceed without verification (as intended)
    s.logger.Debug().
        Uint64("height", height).
        Str("app_hash", hex.EncodeToString(newAppHash)).
        Msg("replayBlock: ExecuteTxs completed (no stored state to verify against)")
} else {
    // State exists, perform verification
    if !bytes.Equal(newAppHash, expectedState.AppHash) {
        // ... error handling for mismatch
    }
}

Please consider refining this error handling to make the replay process more resilient to unexpected storage issues.


// Calculate new state
Expand Down
Loading
Loading