Skip to content
Closed
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
22 changes: 22 additions & 0 deletions block/internal/executing/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,28 @@ func (e *Executor) produceBlock() error {
return fmt.Errorf("failed to apply block: %w", err)
}

// Update header's AppHash if needed and recompute state's LastHeaderHash to match
headerModified := false
switch {
case len(header.AppHash) == 0:
header.AppHash = bytes.Clone(newState.AppHash)
headerModified = true
case bytes.Equal(header.AppHash, newState.AppHash):
// already matches expected state root
case bytes.Equal(header.AppHash, currentState.AppHash):
// header still carries previous state's apphash; update it to the new post-state value
header.AppHash = bytes.Clone(newState.AppHash)
headerModified = true
default:
return fmt.Errorf("header app hash mismatch - got: %x, want: %x", header.AppHash, newState.AppHash)
}

// If we modified the header's AppHash, we need to update the state's LastHeaderHash
// to match the new header hash (since the hash includes AppHash)
if headerModified {
newState.LastHeaderHash = header.Hash()
}

// set the DA height in the sequencer
newState.DAHeight = e.sequencer.GetDAHeight()

Expand Down
5 changes: 5 additions & 0 deletions block/internal/syncing/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,11 @@ func (s *Syncer) trySyncNextBlock(event *common.DAHeightEvent) error {
return fmt.Errorf("failed to apply block: %w", err)
}

// Validate header's AppHash against execution result (if header has an AppHash set)
if len(header.AppHash) != 0 && !bytes.Equal(header.AppHash, newState.AppHash) {
return fmt.Errorf("header app hash mismatch - got: %x, want: %x", header.AppHash, newState.AppHash)
}

// Update DA height if needed
// This height is only updated when a height is processed from DA as P2P
// events do not contain DA height information
Expand Down
29 changes: 19 additions & 10 deletions block/internal/syncing/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,13 @@ func TestProcessHeightEvent_SyncsAndUpdatesState(t *testing.T) {
// Create signed header & data for height 1
lastState := s.getLastState()
data := makeData(gen.ChainID, 1, 0)
_, hdr := makeSignedHeaderBytes(t, gen.ChainID, 1, addr, pub, signer, lastState.AppHash, data, nil)
// Header should have post-execution AppHash (what the producer would set after execution)
postExecAppHash := []byte("app1")
_, hdr := makeSignedHeaderBytes(t, gen.ChainID, 1, addr, pub, signer, postExecAppHash, data, nil)

// Expect ExecuteTxs call for height 1
mockExec.EXPECT().ExecuteTxs(mock.Anything, mock.Anything, uint64(1), mock.Anything, lastState.AppHash).
Return([]byte("app1"), uint64(1024), nil).Once()
Return(postExecAppHash, uint64(1024), nil).Once()

evt := common.DAHeightEvent{Header: hdr, Data: data, DaHeight: 1}
s.processHeightEvent(&evt)
Expand Down Expand Up @@ -240,19 +242,23 @@ func TestSequentialBlockSync(t *testing.T) {
// Sync two consecutive blocks via processHeightEvent so ExecuteTxs is called and state stored
st0 := s.getLastState()
data1 := makeData(gen.ChainID, 1, 1) // non-empty
_, hdr1 := makeSignedHeaderBytes(t, gen.ChainID, 1, addr, pub, signer, st0.AppHash, data1, st0.LastHeaderHash)
// Header should have post-execution AppHash (what the producer would set after execution)
postExecAppHash1 := []byte("app1")
_, hdr1 := makeSignedHeaderBytes(t, gen.ChainID, 1, addr, pub, signer, postExecAppHash1, data1, st0.LastHeaderHash)
// Expect ExecuteTxs call for height 1
mockExec.EXPECT().ExecuteTxs(mock.Anything, mock.Anything, uint64(1), mock.Anything, st0.AppHash).
Return([]byte("app1"), uint64(1024), nil).Once()
Return(postExecAppHash1, uint64(1024), nil).Once()
evt1 := common.DAHeightEvent{Header: hdr1, Data: data1, DaHeight: 10}
s.processHeightEvent(&evt1)

st1, _ := st.GetState(context.Background())
data2 := makeData(gen.ChainID, 2, 0) // empty data
_, hdr2 := makeSignedHeaderBytes(t, gen.ChainID, 2, addr, pub, signer, st1.AppHash, data2, st1.LastHeaderHash)
// Header should have post-execution AppHash (what the producer would set after execution)
postExecAppHash2 := []byte("app2")
_, hdr2 := makeSignedHeaderBytes(t, gen.ChainID, 2, addr, pub, signer, postExecAppHash2, data2, st1.LastHeaderHash)
// Expect ExecuteTxs call for height 2
mockExec.EXPECT().ExecuteTxs(mock.Anything, mock.Anything, uint64(2), mock.Anything, st1.AppHash).
Return([]byte("app2"), uint64(1024), nil).Once()
Return(postExecAppHash2, uint64(1024), nil).Once()
evt2 := common.DAHeightEvent{Header: hdr2, Data: data2, DaHeight: 11}
s.processHeightEvent(&evt2)

Expand Down Expand Up @@ -389,17 +395,20 @@ func TestSyncLoopPersistState(t *testing.T) {
Time: uint64(blockTime.UnixNano()),
},
}
_, sigHeader := makeSignedHeaderBytes(t, gen.ChainID, chainHeight, addr, pub, signer, prevAppHash, emptyData, prevHeaderHash)
// Compute post-execution AppHash (same as DummyExecutor.ExecuteTxs does)
// This is what the producer would set in the header after execution
hasher := sha512.New()
hasher.Write(prevAppHash)
postExecAppHash := hasher.Sum(nil)
_, sigHeader := makeSignedHeaderBytes(t, gen.ChainID, chainHeight, addr, pub, signer, postExecAppHash, emptyData, prevHeaderHash)
evts := []common.DAHeightEvent{{
Header: sigHeader,
Data: emptyData,
DaHeight: daHeight,
}}
daRtrMock.On("RetrieveFromDA", mock.Anything, daHeight).Return(evts, nil)
prevHeaderHash = sigHeader.Hash()
hasher := sha512.New()
hasher.Write(prevAppHash)
prevAppHash = hasher.Sum(nil)
prevAppHash = postExecAppHash
}

// stop at next height
Expand Down
190 changes: 127 additions & 63 deletions execution/evm/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,30 @@ func retryWithBackoffOnPayloadStatus(ctx context.Context, fn func() error, maxRe
return fmt.Errorf("max retries (%d) exceeded for %s", maxRetries, operation)
}

// appendUniqueHash ensures we only keep unique, non-zero hash candidates while
// preserving order so we can try canonical first before falling back to legacy.
func appendUniqueHash(candidates []common.Hash, candidate common.Hash) []common.Hash {
if candidate == (common.Hash{}) {
return candidates
}
for _, existing := range candidates {
if existing == candidate {
return candidates
}
}
return append(candidates, candidate)
}

// buildHashCandidates returns a deduplicated list of hash candidates in the
// order they should be tried.
func buildHashCandidates(hashes ...common.Hash) []common.Hash {
candidates := make([]common.Hash, 0, len(hashes))
for _, h := range hashes {
candidates = appendUniqueHash(candidates, h)
}
return candidates
}

// EngineClient represents a client that interacts with an Ethereum execution engine
// through the Engine API. It manages connections to both the engine and standard Ethereum
// APIs, and maintains state related to block processing.
Expand Down Expand Up @@ -185,42 +209,63 @@ func (c *EngineClient) InitChain(ctx context.Context, genesisTime time.Time, ini
return nil, 0, fmt.Errorf("initialHeight must be 1, got %d", initialHeight)
}

// Acknowledge the genesis block with retry logic for SYNCING status
err := retryWithBackoffOnPayloadStatus(ctx, func() error {
var forkchoiceResult engine.ForkChoiceResponse
err := c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3",
engine.ForkchoiceStateV1{
HeadBlockHash: c.genesisHash,
SafeBlockHash: c.genesisHash,
FinalizedBlockHash: c.genesisHash,
},
nil,
)
if err != nil {
return fmt.Errorf("engine_forkchoiceUpdatedV3 failed: %w", err)
genesisBlockHash, stateRoot, gasLimit, _, err := c.getBlockInfo(ctx, 0)
if err != nil {
return nil, 0, fmt.Errorf("failed to get block info: %w", err)
}

candidates := buildHashCandidates(c.genesisHash, genesisBlockHash, stateRoot)
var selectedGenesisHash common.Hash

for idx, candidate := range candidates {
args := engine.ForkchoiceStateV1{
HeadBlockHash: candidate,
SafeBlockHash: candidate,
FinalizedBlockHash: candidate,
}

// Validate payload status
if err := validatePayloadStatus(forkchoiceResult.PayloadStatus); err != nil {
err = retryWithBackoffOnPayloadStatus(ctx, func() error {
var forkchoiceResult engine.ForkChoiceResponse
err := c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3", args, nil)
if err != nil {
return fmt.Errorf("engine_forkchoiceUpdatedV3 failed: %w", err)
}

if err := validatePayloadStatus(forkchoiceResult.PayloadStatus); err != nil {
c.logger.Warn().
Str("status", forkchoiceResult.PayloadStatus.Status).
Str("latestValidHash", forkchoiceResult.PayloadStatus.LatestValidHash.Hex()).
Interface("validationError", forkchoiceResult.PayloadStatus.ValidationError).
Msg("InitChain: engine_forkchoiceUpdatedV3 returned non-VALID status")
return err
}

return nil
}, MaxPayloadStatusRetries, InitialRetryBackoff, "InitChain")

if err == nil {
selectedGenesisHash = candidate
break
}

if errors.Is(err, ErrInvalidPayloadStatus) && idx+1 < len(candidates) {
c.logger.Warn().
Str("status", forkchoiceResult.PayloadStatus.Status).
Str("latestValidHash", forkchoiceResult.PayloadStatus.LatestValidHash.Hex()).
Interface("validationError", forkchoiceResult.PayloadStatus.ValidationError).
Msg("InitChain: engine_forkchoiceUpdatedV3 returned non-VALID status")
return err
Str("blockHash", candidate.Hex()).
Msg("InitChain: execution engine rejected hash candidate, trying alternate")
continue
}

return nil
}, MaxPayloadStatusRetries, InitialRetryBackoff, "InitChain")
if err != nil {
return nil, 0, err
}
Comment on lines +220 to 259
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic for iterating through hash candidates and retrying engine_forkchoiceUpdatedV3 is duplicated here and in the ExecuteTxs function (lines 369-426). This duplication increases maintenance overhead. Consider extracting this loop into a private helper method to promote code reuse and simplify both functions. The helper could accept the list of candidates and a function to perform the specific forkchoice update logic.


_, stateRoot, gasLimit, _, err := c.getBlockInfo(ctx, 0)
if err != nil {
return nil, 0, fmt.Errorf("failed to get block info: %w", err)
if selectedGenesisHash == (common.Hash{}) {
return nil, 0, fmt.Errorf("execution engine rejected all genesis hash candidates")
}

c.genesisHash = selectedGenesisHash
c.currentHeadBlockHash = selectedGenesisHash
c.currentSafeBlockHash = selectedGenesisHash
c.currentFinalizedBlockHash = selectedGenesisHash
c.initialHeight = initialHeight

return stateRoot[:], gasLimit, nil
Expand Down Expand Up @@ -292,16 +337,12 @@ func (c *EngineClient) ExecuteTxs(ctx context.Context, txs [][]byte, blockHeight
}
txsPayload := validTxs

prevBlockHash, _, prevGasLimit, _, err := c.getBlockInfo(ctx, blockHeight-1)
prevBlockHash, prevBlockStateRoot, prevGasLimit, _, err := c.getBlockInfo(ctx, blockHeight-1)
if err != nil {
return nil, 0, fmt.Errorf("failed to get block info: %w", err)
}

args := engine.ForkchoiceStateV1{
HeadBlockHash: prevBlockHash,
SafeBlockHash: prevBlockHash,
FinalizedBlockHash: prevBlockHash,
}
parentHashCandidates := buildHashCandidates(prevBlockHash, prevBlockStateRoot)

// update forkchoice to get the next payload id
// Create evolve-compatible payloadtimestamp.Unix()
Expand All @@ -325,46 +366,69 @@ func (c *EngineClient) ExecuteTxs(ctx context.Context, txs [][]byte, blockHeight

// Call forkchoice update with retry logic for SYNCING status
var payloadID *engine.PayloadID
err = retryWithBackoffOnPayloadStatus(ctx, func() error {
var forkchoiceResult engine.ForkChoiceResponse
err := c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3", args, evPayloadAttrs)
if err != nil {
return fmt.Errorf("forkchoice update failed: %w", err)
for idx, candidate := range parentHashCandidates {
args := engine.ForkchoiceStateV1{
HeadBlockHash: candidate,
SafeBlockHash: candidate,
FinalizedBlockHash: candidate,
}

// Validate payload status
if err := validatePayloadStatus(forkchoiceResult.PayloadStatus); err != nil {
c.logger.Warn().
Str("status", forkchoiceResult.PayloadStatus.Status).
Str("latestValidHash", forkchoiceResult.PayloadStatus.LatestValidHash.Hex()).
Interface("validationError", forkchoiceResult.PayloadStatus.ValidationError).
Uint64("blockHeight", blockHeight).
Msg("ExecuteTxs: engine_forkchoiceUpdatedV3 returned non-VALID status")
return err
}
err = retryWithBackoffOnPayloadStatus(ctx, func() error {
var forkchoiceResult engine.ForkChoiceResponse
err := c.engineClient.CallContext(ctx, &forkchoiceResult, "engine_forkchoiceUpdatedV3", args, evPayloadAttrs)
if err != nil {
return fmt.Errorf("forkchoice update failed: %w", err)
}

if forkchoiceResult.PayloadID == nil {
c.logger.Error().
Str("status", forkchoiceResult.PayloadStatus.Status).
Str("latestValidHash", forkchoiceResult.PayloadStatus.LatestValidHash.Hex()).
Interface("validationError", forkchoiceResult.PayloadStatus.ValidationError).
Interface("forkchoiceState", args).
Interface("payloadAttributes", evPayloadAttrs).
Uint64("blockHeight", blockHeight).
Msg("returned nil PayloadID")
// Validate payload status
if err := validatePayloadStatus(forkchoiceResult.PayloadStatus); err != nil {
c.logger.Warn().
Str("status", forkchoiceResult.PayloadStatus.Status).
Str("latestValidHash", forkchoiceResult.PayloadStatus.LatestValidHash.Hex()).
Interface("validationError", forkchoiceResult.PayloadStatus.ValidationError).
Uint64("blockHeight", blockHeight).
Msg("ExecuteTxs: engine_forkchoiceUpdatedV3 returned non-VALID status")
return err
}

if forkchoiceResult.PayloadID == nil {
c.logger.Error().
Str("status", forkchoiceResult.PayloadStatus.Status).
Str("latestValidHash", forkchoiceResult.PayloadStatus.LatestValidHash.Hex()).
Interface("validationError", forkchoiceResult.PayloadStatus.ValidationError).
Interface("forkchoiceState", args).
Interface("payloadAttributes", evPayloadAttrs).
Uint64("blockHeight", blockHeight).
Msg("returned nil PayloadID")

return fmt.Errorf("returned nil PayloadID - (status: %s, latestValidHash: %s)",
forkchoiceResult.PayloadStatus.Status,
forkchoiceResult.PayloadStatus.LatestValidHash.Hex())
}

payloadID = forkchoiceResult.PayloadID
return nil
}, MaxPayloadStatusRetries, InitialRetryBackoff, "ExecuteTxs forkchoice")

return fmt.Errorf("returned nil PayloadID - (status: %s, latestValidHash: %s)",
forkchoiceResult.PayloadStatus.Status,
forkchoiceResult.PayloadStatus.LatestValidHash.Hex())
if err == nil {
break
}

if errors.Is(err, ErrInvalidPayloadStatus) && idx+1 < len(parentHashCandidates) {
c.logger.Warn().
Str("blockHash", candidate.Hex()).
Uint64("blockHeight", blockHeight-1).
Msg("ExecuteTxs: execution engine rejected parent hash candidate, trying alternate")
continue
}

payloadID = forkchoiceResult.PayloadID
return nil
}, MaxPayloadStatusRetries, InitialRetryBackoff, "ExecuteTxs forkchoice")
if err != nil {
return nil, 0, err
}

if payloadID == nil {
return nil, 0, fmt.Errorf("engine returned nil PayloadID after trying %d parent hash candidates", len(parentHashCandidates))
}

// get payload
var payloadResult engine.ExecutionPayloadEnvelope
err = c.engineClient.CallContext(ctx, &payloadResult, "engine_getPayloadV4", *payloadID)
Expand Down
38 changes: 38 additions & 0 deletions execution/evm/execution_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,41 @@ func TestRetryWithBackoffOnPayloadStatus_WrappedRPCErrors(t *testing.T) {
// Should fail immediately without retries on non-syncing errors
assert.Equal(t, 1, attempts, "expected exactly 1 attempt, got %d", attempts)
}

func TestBuildHashCandidates(t *testing.T) {
t.Parallel()

hashA := common.HexToHash("0x01")
hashB := common.HexToHash("0x02")
var zeroHash common.Hash

tests := []struct {
name string
hashes []common.Hash
expects []common.Hash
}{
{
name: "deduplicates while preserving order",
hashes: []common.Hash{hashA, hashB, hashA},
expects: []common.Hash{hashA, hashB},
},
{
name: "skips zero hash",
hashes: []common.Hash{zeroHash, hashB},
expects: []common.Hash{hashB},
},
{
name: "handles empty input",
hashes: []common.Hash{},
expects: []common.Hash{},
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.expects, buildHashCandidates(tt.hashes...))
})
}
}
Loading
Loading