diff --git a/block/internal/executing/executor.go b/block/internal/executing/executor.go index f376a0cb0..b4f5c32f1 100644 --- a/block/internal/executing/executor.go +++ b/block/internal/executing/executor.go @@ -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() diff --git a/block/internal/syncing/syncer.go b/block/internal/syncing/syncer.go index 812b40b20..3f0c43791 100644 --- a/block/internal/syncing/syncer.go +++ b/block/internal/syncing/syncer.go @@ -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 diff --git a/block/internal/syncing/syncer_test.go b/block/internal/syncing/syncer_test.go index 21b012cf2..ea4e36bb4 100644 --- a/block/internal/syncing/syncer_test.go +++ b/block/internal/syncing/syncer_test.go @@ -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) @@ -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) @@ -389,7 +395,12 @@ 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, @@ -397,9 +408,7 @@ func TestSyncLoopPersistState(t *testing.T) { }} 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 diff --git a/execution/evm/execution.go b/execution/evm/execution.go index 33b466c05..6a23d0f1d 100644 --- a/execution/evm/execution.go +++ b/execution/evm/execution.go @@ -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. @@ -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 } - _, 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 @@ -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() @@ -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) diff --git a/execution/evm/execution_status_test.go b/execution/evm/execution_status_test.go index a25bb539e..ef6caf890 100644 --- a/execution/evm/execution_status_test.go +++ b/execution/evm/execution_status_test.go @@ -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...)) + }) + } +} diff --git a/types/state.go b/types/state.go index bf535de70..2c8079276 100644 --- a/types/state.go +++ b/types/state.go @@ -78,9 +78,5 @@ func (s State) AssertValidForNextState(header *SignedHeader, data *Data) error { if !bytes.Equal(header.LastHeaderHash, s.LastHeaderHash) { return fmt.Errorf("invalid last header hash - got: %x, want: %x", header.LastHeaderHash, s.LastHeaderHash) } - if !bytes.Equal(header.AppHash, s.AppHash) { - return fmt.Errorf("invalid last app hash - got: %x, want: %x", header.AppHash, s.AppHash) - } - return nil } diff --git a/types/state_test.go b/types/state_test.go index e07342c16..326ee3d12 100644 --- a/types/state_test.go +++ b/types/state_test.go @@ -131,28 +131,6 @@ func TestAssertValidForNextState(t *testing.T) { data: &Data{}, expectedError: "invalid last header hash", }, - "app hash mismatch": { - state: State{ - ChainID: "test-chain", - LastHeaderHash: []byte("expected-hash"), - LastBlockHeight: 1, - LastBlockTime: now, - AppHash: []byte("expected-app-hash"), - }, - header: &SignedHeader{ - Header: Header{ - BaseHeader: BaseHeader{ - ChainID: "test-chain", Height: 2, - Time: nowUnixNano, - }, - DataHash: dataHashForEmptyTxs, - LastHeaderHash: []byte("expected-hash"), - AppHash: []byte("wrong-app-hash"), - }, - }, - data: &Data{}, - expectedError: "invalid last app hash", - }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) {