fix(ethrex-engine): build payloads on the parent consensus expects

A 3-node devnet with the embedded EL never finalized: 22 `parent_hash mismatch`
errors, peer imports halved, no aggregation coverage. A consensus-only control run
on the same image finalized normally, isolating the fault to the integration.

The state transition requires

    payload.parent_hash == state.latest_execution_payload_header.block_hash

which is the parent the consensus chain expects. build_payload instead derived it
from store.get_latest_canonical_block_hash(), the node's *own* EL head. Each node
runs an independent in-memory execution layer, so those two drift apart, and a
proposer's payload then named a parent no peer agreed with — every node's STF
rejected the block. It failed silently from the proposer's side: no EL rejection,
no warning, the block just never stuck.

The Engine-API path did not have this bug, because its build-mode
forkchoiceUpdated pointed the EL at el_hash_at(store.head()) before building.
Collapsing that two-step into a single call dropped the step that chose the parent.

build_payload now takes parent_el_hash explicitly and re-points the EL at that
block before building; el_integration passes el_hash_at(head_root). safe and
finalized are left unset in that fork-choice call, since pinning them would forbid
a later build on an earlier block.

Regression test builds_on_the_requested_parent_not_the_el_head advances the EL two
blocks, then builds on block 1 and asserts the payload names that parent; it fails
against the previous code. Unit tests could not have caught this — it needs more
than one execution layer to appear.

Verified: 3 nodes, 36 slots, finalized at slot 40 with all nodes on the same
finalized root and 43 payloads executed each; zero parent_hash mismatches, zero
synthetic fallbacks, zero panics. Matches the consensus-only control (slot 41).

Author: pablodeymo

crates/blockchain/src/el_integration.rs

@@ -85,8 +85,15 @@ impl BlockChainServer {
         let engine = self.execution_engine.as_ref()?;
         let head_root = self.store.head().unwrap_or_default();
         let genesis_time = self.store.config().genesis_time;
+        // Build on the EL block the *consensus* chain expects to be extended —
+        // the head block's own payload hash — not whatever this node's EL happens
+        // to have as its head. The state transition checks the new payload's
+        // `parent_hash` against `state.latest_execution_payload_header.block_hash`,
+        // so a drifted EL head yields a block every peer rejects.
+        let parent_el_hash = self.el_hash_at(head_root);
         engine
             .build_payload(
+                parent_el_hash,
                 compute_time_at_slot(genesis_time, slot),
                 // Zero until Lean defines a RANDAO mix.
                 H256::ZERO,

crates/net/ethrex-engine/src/lib.rs

@@ -112,27 +112,42 @@ impl EthrexEngine {
         Ok(self.store.get_latest_block_number().await?)
     }
 
-    /// Build the execution payload for a block being proposed on top of the
-    /// current canonical head.
+    /// Build the execution payload for a block being proposed on top of
+    /// `parent_el_hash`.
     ///
     /// One call: ethrex creates the payload skeleton and fills it synchronously,
     /// so unlike the Engine API there is no id to hold onto and no second fetch.
     ///
+    /// `parent_el_hash` **must** be the EL block hash the consensus chain expects
+    /// to be extended — the `execution_payload.block_hash` of the Lean block
+    /// being built on. It is passed in rather than read from this engine's own
+    /// canonical head because the two can differ: every node runs its own
+    /// execution layer, and an EL head that has drifted from the consensus chain
+    /// would produce a payload whose `parent_hash` fails the state transition's
+    /// check against `state.latest_execution_payload_header.block_hash` — which
+    /// makes every peer reject the block.
+    ///
     /// `beacon_root` follows the lean-parent-root convention — it is the
     /// proposed block's `parent_root`, and must be the same value later passed
     /// to [`Self::execute_payload`], or the EL's block-hash check fails.
     pub async fn build_payload(
         &self,
+        parent_el_hash: LeanH256,
         timestamp: u64,
         prev_randao: LeanH256,
         beacon_root: LeanH256,
         fee_recipient: [u8; 20],
     ) -> Result<ExecutionPayloadV3, EngineError> {
-        let parent = self
-            .store
-            .get_latest_canonical_block_hash()
-            .await?
-            .ok_or(EngineError::NoCanonicalHead)?;
+        let parent = H256(parent_el_hash.0);
+        // Make the EL treat that block as its head before building on it, so the
+        // payload is produced against the state the consensus chain expects.
+        // safe/finalized are left unset (ethrex reads zero as "not provided"):
+        // pinning them here would forbid a later build on an earlier block.
+        apply_fork_choice(&self.store, parent, H256::zero(), H256::zero())
+            .await
+            .map_err(|err| {
+                EngineError::Conversion(format!("cannot build on parent {parent:#x}: {err}"))
+            })?;
         let args = BuildPayloadArgs {
             parent,
             timestamp,

crates/net/ethrex-engine/tests/roundtrip.rs

@@ -28,6 +28,7 @@ async fn builds_executes_and_advances_head() {
 
     let payload = engine
         .build_payload(
+            genesis_hash,
             genesis_timestamp + 12,
             LeanH256::ZERO,
             genesis_hash,
@@ -68,6 +69,7 @@ async fn rejects_payload_with_mismatched_beacon_root() {
 
     let payload = engine
         .build_payload(
+            genesis_hash,
             genesis_timestamp + 12,
             LeanH256::ZERO,
             genesis_hash,
@@ -82,3 +84,59 @@ async fn rejects_payload_with_mismatched_beacon_root() {
         "a payload replayed under a different beacon root must not be accepted"
     );
 }
+
+/// The payload must be built on the parent the caller names, not on whatever
+/// this engine's own canonical head happens to be.
+///
+/// Every node runs its own execution layer, so a proposer's EL head can drift
+/// from the consensus chain. The state transition checks a new payload's
+/// `parent_hash` against `state.latest_execution_payload_header.block_hash`, so
+/// building on the wrong parent produces a block every peer rejects — which
+/// stalls finality without any error surfacing locally.
+#[tokio::test]
+async fn builds_on_the_requested_parent_not_the_el_head() {
+    let (engine, genesis_timestamp) = engine().await;
+    let genesis_hash = engine.head_hash().await.unwrap();
+
+    // Advance the EL two blocks, so its head is no longer genesis.
+    let mut parent = genesis_hash;
+    let mut block_1 = LeanH256::ZERO;
+    for i in 1..=2u64 {
+        let payload = engine
+            .build_payload(
+                parent,
+                genesis_timestamp + 12 * i,
+                LeanH256::ZERO,
+                parent,
+                [0u8; 20],
+            )
+            .await
+            .expect("build payload");
+        engine.execute_payload(&payload, parent).expect("execute");
+        parent = payload.block_hash;
+        if i == 1 {
+            block_1 = parent;
+        }
+        engine.set_head(parent, parent, genesis_hash).await.unwrap();
+    }
+    assert_eq!(engine.head_number().await.unwrap(), 2, "EL head advanced");
+
+    // Now ask for a payload extending block 1 (NOT the EL head at block 2), the
+    // way a proposer would after a reorg or when its EL ran ahead.
+    let payload = engine
+        .build_payload(
+            block_1,
+            genesis_timestamp + 999,
+            LeanH256::ZERO,
+            block_1,
+            [0u8; 20],
+        )
+        .await
+        .expect("build on an explicit non-head parent");
+
+    assert_eq!(
+        payload.parent_hash, block_1,
+        "payload must name the requested parent, not the EL's own head"
+    );
+    assert_eq!(payload.block_number, 2, "extending block 1 yields height 2");
+}

docs/plans/scope-down-review.md

@@ -0,0 +1,195 @@
+# Review guide: in-process ethrex, scoped down
+
+What to look at, what to be suspicious of, and what is still unfinished.
+
+**Branch:** `feat/ethrex-inprocess` — one commit (`134dcb4`) off `origin/main` (`b4a8f78`).
+**Not pushed yet.** Nothing is force-pushed and PR #530 is untouched pending your call (§7).
+
+---
+
+## 1. What this is
+
+Run the execution layer **in-process**: ethrex linked in as a library, driven by
+direct function calls. One binary, no Engine API, no JSON-RPC, no JWT.
+
+Per your scoping call, all out-of-process machinery was removed. That work still
+exists as PR #367, so nothing is lost — this branch simply stops overlapping it.
+
+| | Previous PR #530 | This branch |
+|---|---|---|
+| Commits | 12 (3 merges of main, plus #367 absorbed) | **1**, off current main |
+| Files changed | ~80 | **43** |
+| Engine-API code | ~790 lines (JWT, JSON-RPC client, wire test) | **0** |
+| EL interface | `ExecutionEngine` trait, Engine-API methods, `PayloadId`, payload cache, wire types | **3 direct methods** |
+| CLI | `--execution-mode` + 3 external flags | **`--el-genesis`** |
+
+Diff: 43 files, +3444 / −779.
+
+## 2. Suggested reading order
+
+Reviewing in this order means each file makes sense before you reach its callers.
+
+1. `crates/net/ethrex-engine/src/lib.rs` — **the whole EL surface**, three methods.
+   Read this first; everything else is wiring.
+2. `crates/net/ethrex-engine/src/conversion.rs` — the payload ⇄ block mapping. The
+   only genuinely fiddly code; check the field table in the guide against it.
+3. `crates/blockchain/src/el_integration.rs` — the four actor hooks and the
+   never-stall-consensus policy.
+4. `crates/blockchain/src/lib.rs` — where those hooks attach to the tick loop
+   (interval 0 head update, interval 4 build, gossip import).
+5. `bin/ethlambda/src/main.rs` — engine construction and the **genesis seeding**
+   (§4, decision 3).
+6. `crates/blockchain/state_transition/src/execution_payload.rs` and the type
+   changes — the consensus-side schema (§5).
+7. Everything else is test literals, docs and tooling.
+
+## 3. The claim most worth challenging
+
+**Some code that arrived via #367 stays, and it is not Engine-API code.**
+
+| Kept | Why it is required in-process |
+|---|---|
+| `ExecutionPayloadV3` in `BlockBody` | The proposer embeds the payload so **peers execute it in their own embedded EL**. Without it, no peer can replicate execution. |
+| `process_execution_payload` (STF) | Validates the payload's parent hash and slot timestamp on import. |
+| `latest_execution_payload_header` in `State` / `StateDiff` | Reconstructed states must keep the EL block-hash chain, or the parent-hash check breaks after a diff replay. |
+| `State::from_genesis_with_el_hash` | Seeds the consensus genesis with the EL genesis hash. |
+
+If you disagree that these belong here, that is the conversation to have — it is
+the one place where "only in-process changes" is a judgement call rather than a
+mechanical deletion.
+
+## 4. Decisions to scrutinise
+
+Each is reversible; the cost of reversing is noted.
+
+**1. Direct API instead of the `ExecutionEngine` trait.** (your D1=B)
+`build_payload` / `execute_payload` / `set_head`. This deleted `PayloadId`, the
+`Mutex<HashMap<[u8;8], _>>` payload cache, and the build-then-fetch two-step —
+all artefacts of the Engine API being stateless and networked.
+*Reversing:* reintroduce the trait, which #367 already contains.
+
+**2. No fee-recipient configuration.** ← *the one I am least sure about*
+#367 read `suggested_fee_recipient` from `validator-config.yaml`; main has no such
+plumbing. Rather than re-add config parsing for something the integration does not
+need, the EL is handed the zero address with a comment. Lean has no fee market or
+block rewards, so nothing is being directed anywhere.
+*Reversing:* ~20 lines — a config field, a hex parser, and one more `BlockChainConfig` field.
+
+**3. The EL genesis hash is derived, not configured.**
+The engine bootstraps from `--el-genesis`, so its startup head *is* the EL genesis
+block; `main.rs` reads it back and seeds the consensus anchor. The external path
+needed a flag because the EL was a separate process.
+*Why it matters:* forgetting this seed fails **silently** — consensus looks healthy
+while the EL sits frozen at genesis and every proposal falls back to a synthetic
+payload. Worth confirming you find the derivation trustworthy.
+
+**4. `execute_payload` is synchronous.**
+`Blockchain::add_block` is a sync ethrex call, so the gossip-import path no longer
+awaits. Simpler, but it does mean EL execution happens on the actor thread.
+*Consider:* whether block execution time on the actor is acceptable, or whether it
+should move off-thread later.
+
+**5. Single ethrex revision across the workspace.**
+`crates/net/p2p` was pinned to an older ethrex for ENR parsing; it now follows the
+workspace revision, which required porting `parse_enrs` to v15's typed
+`NodeRecord`. This touches a crate unrelated to the feature.
+*Why it is not optional:* `ethrex-crypto` bundles a C SHA3 with non-namespaced
+symbols, so two ethrex versions multiply-define them under GNU `ld`. macOS `ld64`
+tolerates it — it only fails in the Linux release build.
+
+**6. In-memory EL store.** EL state resets on restart. Fine for a PoC; persistence
+is an `ethrex-storage` feature away and pairs with EL-aware checkpoint sync.
+
+**7. Mock-EL test seam dropped.** (your D4) No trait means nothing to mock; the
+engine tests drive a real embedded ethrex instead.
+
+## 5. Consensus-path changes to check carefully
+
+These touch the tick loop, so they deserve more attention than the rest:
+
+- **Interval 4** — `build_execution_payload` runs inline, immediately before the
+  block is assembled. Failure returns `None` and `build_block` falls back to
+  `synthetic_payload`.
+- **Interval 0** — `notify_execution_layer` updates the EL head, spawned
+  fire-and-forget.
+- **Gossip import** — `import_gossiped_block` executes the payload *before* the
+  store sees the block. A rejection drops the block; anything else proceeds.
+- **Own block** — after building, we execute our own payload, because nobody
+  gossips it back to us and the EL head would otherwise never advance.
+
+The invariant throughout: **the execution layer never stalls consensus.** Only an
+explicit rejection of a received payload drops a block; every other failure logs
+and continues.
+
+## 6. Verification status
+
+| Check | Status |
+|---|---|
+| `cargo build --workspace` | ✅ clean |
+| `cargo clippy --workspace --all-targets -- -D warnings` | ✅ clean |
+| `cargo fmt --all --check` | ✅ clean |
+| Tests (blockchain, state-transition, engine, bin, p2p) | ✅ **299 passed, 0 failed** |
+| Engine roundtrip + beacon-root rejection tests | ✅ pass |
+| 3-node devnet **with** the embedded EL | ✅ finalized at slot 40 |
+| 3-node devnet **without** the EL (control) | ✅ finalized at slot 41 |
+
+The EL-enabled run matches the consensus-only control, so the execution layer
+costs nothing in liveness. All three nodes agreed on the same finalized root, and
+each executed exactly 43 payloads — lockstep.
+
+### 6.1 Bug found by the devnet and fixed: `parent_hash mismatch`
+
+Worth reading, because it is the one real defect the scope-down introduced and no
+unit test could have caught it — it only appears with **multiple independent
+execution layers**.
+
+**Symptom.** With the EL enabled the chain never finalized: 22 `parent_hash
+mismatch` errors, peer imports halved (13 vs 30), no aggregation coverage, no
+finality. Silent from the proposer's side — zero EL rejections, zero warnings.
+The block simply did not stick anywhere.
+
+**Cause.** The state transition requires
+
+```
+payload.parent_hash == state.latest_execution_payload_header.block_hash
+```
+
+— the parent the *consensus chain* expects. `build_payload` instead derived the
+parent from `store.get_latest_canonical_block_hash()`, this node's *own* EL head.
+With three independent ELs those drift apart, so a proposer's payload named the
+wrong parent and every node's STF rejected the block.
+
+#367 did not have this bug: its build-mode `forkchoiceUpdated` pointed the EL at
+`el_hash_at(store.head())` before building. Collapsing that two-step into one call
+dropped the step that set the parent.
+
+**Fix.** `build_payload` takes `parent_el_hash` explicitly; `el_integration` passes
+`el_hash_at(head_root)` — the consensus head's payload hash — and the engine
+re-points the EL at that block before building. safe/finalized are deliberately
+left unset there: pinning them would forbid a later build on an earlier block.
+
+**Regression test.** `builds_on_the_requested_parent_not_the_el_head` advances the
+EL two blocks, then asks for a payload extending block 1 and asserts the payload
+names *that* parent. It fails against the old code.
+
+**Method note.** The first hypothesis — that `import_gossiped_block` was dropping
+blocks on EL rejection — was wrong, and the logs disproved it (zero rejections)
+before any code was changed.
+
+## 7. Open questions for you
+
+1. **Publishing.** Force-push this onto `feat/ethrex-inprocess-poc` (keeps PR #530
+   and its discussion) or push `feat/ethrex-inprocess` as a new PR and close #530?
+   Force-push rewrites the remote branch, so it needs your say-so.
+2. **Decision 2** — fee-recipient config: leave dropped, or restore it?
+3. **Decision 4** — EL execution on the actor thread: acceptable for now?
+4. Anything in §3 you think should not be in this PR.
+
+## 8. Known remaining work
+
+- The two published artifacts still describe the trait / two-mode design and need
+  updating once the code settles.
+- `docs/plans/scope-down-to-inprocess.md` (the proposal) and this file can both be
+  dropped from the PR if you would rather not carry planning docs.
+- Prague / `ExecutionPayloadV4` support is out of scope; the EL genesis must be
+  Cancun.

scripts/inprocess-devnet/run.sh

@@ -29,6 +29,7 @@ TRACE=false
 KEEP=false
 BUILD=false
 VERIFY=true
+NO_EL=false
 
 KEYGEN_IMAGE="blockblaz/hash-sig-cli:latest"
 GENESIS_IMAGE="ethpandaops/eth-beacon-genesis:pk910-leanchain"
@@ -57,6 +58,7 @@ while [[ $# -gt 0 ]]; do
     --keep)        KEEP=true; shift ;;
     --build)       BUILD=true; shift ;;
     --no-verify)   VERIFY=false; shift ;;
+    --no-el)       NO_EL=true; shift ;;
     -h|--help)     sed -n '2,20p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//'; exit 0 ;;
     *) echo "unknown option: $1 (try --help)" >&2; exit 2 ;;
   esac
@@ -279,7 +281,7 @@ for ((i = 0; i < NODES; i++)); do
     --http-address 0.0.0.0 \
     --metrics-port "$((8081 + i))" \
     --api-port "$((15052 + i))" \
-    --el-genesis /config/el-genesis.json \
+    $([[ "$NO_EL" == false ]] && echo "--el-genesis /config/el-genesis.json") \
     $([[ $i -eq 0 ]] && echo "--is-aggregator") >/dev/null || die "failed to start $NAME"
   ok "$NAME (quic $((9001 + i)), api $((15052 + i)))$([[ $i -eq 0 ]] && echo ' [aggregator]')"
 done
@@ -323,8 +325,9 @@ count1() { local n; n=$(grep -c "$1" "$2" 2>/dev/null || true); echo "${n:-0}";
 FAIL=0
 
 # 1. the embedded EL came up on every node
-EL_UP=$(count "In-process ethrex execution engine enabled")
-if [[ "$EL_UP" == "$NODES" ]]; then ok "in-process EL enabled on $EL_UP/$NODES node(s)"
+EL_UP=$(count "Embedded ethrex enabled")
+if [[ "$NO_EL" == true ]]; then ok "consensus-only control run (no EL expected)"
+elif [[ "$EL_UP" == "$NODES" ]]; then ok "in-process EL enabled on $EL_UP/$NODES node(s)"
 else warn "in-process EL enabled on $EL_UP/$NODES node(s)"; FAIL=1; fi
 
 # 2. blocks were produced (works with a single node, unlike the import path)
@@ -345,9 +348,11 @@ if [[ -n "$FINAL" ]]; then ok "${FINAL#*Checkpoint finalized }"
 else warn "no finalization yet (needs ~30 slots; ran $SLOTS)"; fi
 
 # 5. the EL actually built and executed payloads (trace-level: needs --trace)
-if [[ "$TRACE" == true ]]; then
+if [[ "$NO_EL" == true ]]; then
+  warn "consensus-only control run (--no-el): EL checks skipped"
+elif [[ "$TRACE" == true ]]; then
   BUILT=$(count "Built execution payload")
-  EXECD=$(( $(count "newPayload on own-built block") + $(count "newPayload ok") ))
+  EXECD=$(count "EL executed payload")
   if (( BUILT > 0 )); then ok "EL payloads built: $BUILT"
   else warn "no EL payload builds"; FAIL=1; fi
   if (( EXECD > 0 )); then ok "EL payloads submitted for execution: $EXECD"
@@ -357,7 +362,7 @@ else
 fi
 
 # 6. red flags
-BAD=$(( $(count "falling back to synthetic") + $(count "getPayload failed") + $(count "rejected payload") ))
+BAD=$(( $(count "using synthetic payload") + $(count "EL rejected payload") ))
 if (( BAD == 0 )); then ok "no synthetic fallbacks / rejected payloads"
 else warn "EL failure lines: $BAD"; FAIL=1; fi