Add Forkchoice changes for ePBS by Subhasish-Behera · Pull Request #502 · grandinetech/grandine

Subhasish-Behera · 2025-12-01T03:10:24Z

Major changes:
The whole logic is based on the idea that it doesn't matter how you reach the segment-level identifiers. i.e though ePBS introduces a separate envelope object, but the same segment structure can be followed. So the difference between a envelope(full block)/ empty block is which state is read from that block. One of the ways to do that is by following the existing pattern of keeping a chunk of the Store fields inside Chainlink

ChainLink dual statefield

  block_state: Option<Arc<BeaconState<P>>>,           // Pre-execution (after beacon block STF)
  execution_payload_state: Option<Arc<BeaconState<P>>>, // Post-execution (after payload STF)

The changed structure of empty/full block pretty much behave as siblings if it is looked from view of part of forkchoice calculation which depends on reading parent information.

with epbs, to get correct forchoice results, its important to know both which variant the block itself is + which block will it be effecting(for eg: not just the insertion of envelope/empty block have a type but also what it is inserted into has a type i.e the parent block can be full as well as empty)
so the the most common structure that is going to happen is:

 Segment 0:                ──► [Parent X]
                                │
                                ├──► Segment 1: [Empty A] ──► [Empty B]
                                │                  ▲
                                │                  │
                                │    unfinalized_locations_empty["A"] = (seg:1, pos:0)
                                │
                                └──► Segment 2: [Full A]
                                                 ▲
                                                 │
                                     unfinalized_locations_full["A"] = (seg:2, pos:0)

both weight propagation purposes as well as the insertion purpose of full/empty block of arrival, the choice of introducing dual maps is much easier than differentiation at unfinalized_block level or using chain_link fields.
the key currently for both the map is block_root. A previous approach was made to keep unfinalized_locations_full maps key as execution_hash and unfinalized_locations_empty through block_root. But there was significant overhead in terms of number of conversion calla/reverse lookup for hash to root and viceversa. plus as only one variant of full/empty per blockroot can reach till forkchoice, there is ideally no problem with having the same keys.
a way to move forward with the changes is to look at in which scenarios(insertion into segment, calculation supporting vote, tiebraker , weight propagtion/childern filter/setting ForkChoicePayloadStatus) empty vs full is preferred across spec and current code.

povi · 2025-12-12T14:01:52Z

fork_choice_control/src/mutator.rs

    ) -> Result<()> {
        match *result {
            Ok(BlockAction::Accept(mut chain_link, attester_slashing_results)) => {
+                eprintln!("DEBUG: BlockAction::Accept for root={:?}", block_root);


Remove eprintln! from PR, if you want to add permanent debug info to fork choice, use trace_with_peers!

povi · 2025-12-12T14:03:58Z

fork_choice_store/src/store.rs

+    ///
+    /// This uses get_location() which prefers full variant when both exist.
+    #[must_use]
+    pub fn chain_link_prefer_full(&self, block_root: H256) -> Option<&ChainLink<P>> {


Method body is the same as Store::chain_link. Remove the duplicate

Code still contains duplicate methods with different names

povi · 2025-12-12T14:05:23Z

fork_choice_store/src/store.rs

+    /// 3. Both maps keyed by same beacon_block_root, so no conversion needed
+    #[must_use]
+    fn get_location(&self, block_root: H256) -> Option<Location> {
+        // Check full variant first (prefer it if exists)


Can we do that? What if validators vote for node with no payload and the node with no payload has bigger weight?

Subhasish-Behera · 2025-12-12T14:49:52Z

@povi , this PR is currently a bit hacky,the get_location usage is not correct. using it everywhere rightnow gives wrong result as the priority is fixed in get location. so i have a updated version which works better locally. I was waiting for ethereum/consensus-specs#4489 to be merged as running it locally revealed the right flow. But as you have started reviewing, I will update this soon.

Subhasish-Behera · 2026-02-02T12:01:54Z

hey @povi I have updated the PR for 1.7 spec. please give it a review

fork_choice_store/src/store.rs

fork_choice_control/src/queries.rs

povi · 2026-02-11T14:39:51Z

fork_choice_control/src/controller.rs

    }

+    /// Check if execution payload envelope has been seen for block_root.
+    pub fn has_envelope(&self, block_root: H256) -> bool {


Duplicate method definition in Controller

povi · 2026-02-11T14:40:08Z

fork_choice_control/src/controller.rs

+    }
+
+    /// Check if blob data is available for block_root.
+    pub fn is_data_available(&self, block_root: H256) -> bool {


Duplicate method definition in Controller

povi · 2026-02-11T14:40:12Z

fork_choice_control/src/controller.rs

+    }
+
+    /// Get the slot of a block by its root.
+    pub fn block_slot(&self, block_root: H256) -> Option<Slot> {


Duplicate method definition in Controller

fork_choice_control/src/queries.rs

fork_choice_control/src/spec_tests.rs

povi · 2026-02-11T14:42:04Z

I won't proceed with reviewing since the code doesn't even compile, even fork_choice_control module doesn't (that contains test runner for EF consensus spec tests). E.g. there are duplicate definitions of Controller methods, like has_envelope, is_data_available and block_slot; there are calls to non-existing methods etc.
Other than that, there are plenty of questionable code blocks that either show this is either sloppy work or generated code.

Subhasish-Behera · 2026-02-11T15:06:52Z

Hey @povi, yeah, it has a currently duplicate block(has_envelope, is_data_available , block_slot ) because I added that before hangleang but his was merged earlier(but while resolving the conflicts i missed the duplicate funcitons) and was using cargo check for the PR and not cargo build while pushing (and unused functions because I deleted inline test helpers which were required like assert_head_payload_status because they were required to test those payload extension scenarios but not part of the main flow(and the spec tests havent introduced those yet)) . I am currently running a fixed version of it with kurtosis and geth with self build enabled. its working for a single node and 2 nodes setup.
But with the new patch of full block creation, 1 of the test (spec_tests::gloas_minimal_withholding_consensus_spec_tests_tests_minimal_gloas_fork_choice_withholding_pyspec_tests_withholding_attack_unviable_honest_chain) which i am trying to fix now for the actual update

povi · 2026-02-11T21:40:22Z

which i am trying to fix now for the actual update

Then what is this that you requested me to review? Reviewing takes time and effort in large PR like this. If you don't even bother to review your own changes for obvious logical errors/code style/code validity (any of these) then it is plainly unacceptable, very much so for core part of the application.

Subhasish-Behera · 2026-02-16T21:44:17Z

command to run kurtosis: kurtosis run github.com/ethpandaops/ethereum-package --args-file network_params_spamoor.yaml --enclave grandine-local-testnet

network_params_spamoor.yaml: 
  participants:                                                                                                       
    - el_type: geth                                                                                                   
      el_image: greendoor/geth-bal-devnet-patched:latest                        
      cl_type: grandine
      cl_image: greendoor/grandine:local                                        
      cl_extra_params:
        - --subscribe-all-subnets
        - --subscribe-all-data-column-subnets
        - --enable-payload-build
        - --features=TrustOwnBlockSignatures
      count: 2

  network_params:
    electra_fork_epoch: 0
    gloas_fork_epoch: 2
    seconds_per_slot: 12
    preset: minimal

  global_log_level: debug

  snooper_enabled: false

  additional_services:
    - dora
    - spamoor

  spamoor_params:
    spammers:
      - scenario: eoatx
        config:
          throughput: 2
      - scenario: blobs
        config:
          throughput: 1

use a patched version of geth: https://github.com/Subhasish-Behera/go-ethereum/tree/bal-devnet-2_patched(use make geth to make own binary) without the access list , slot number to use engine_getPayloadV5 as targeted in devnet-0. Both the images are public. in local runs, was able to finalise over multiple epochs with having nodes non-optimistic. Note: Dora isn't showing the status correctly for gloas but using curl or kurtosis logs would be fine. Also seen cases of enr mismatches when gloas epoch to 1(too early)
Also works with ethpandaops/geth:epbs-devnet-0

currently dosent show any forkchoice tests fails at cargo test --profile compact --features default-networks --workspace --exclude zkvm_host --exclude zkvm_guest_risc0
builds with: cargo build --profile compact --features default-networks --workspace --exclude zkvm_host --exclude zkvm_guest_risc0

Subhasish-Behera · 2026-02-18T10:11:47Z

hey @povi I have updated the PR, the commit 460071a passes build, tests as well as kurtosis local setup
but accepted branch code still had issues that broke Kurtosis liveness (stalled state) or dropped
peer score to zero.

This commit apart from forkchoice adaptations, adds the implementation to fix those potential bugs(along with a few other bugs):

Self-build state mismatch in compute_execution_payload_envelope (incorrect state usage).
incorrect data.index handling in attestation packing/ pool conversion (now keeps committee index separate and restores attestation data.index correctly).

Also uses the same format of engine_getPayloadV5 to be used in devnet-0 (ref: https://discord.com/channels/595666850260713488/874767108809031740/1470849927402754262)

Subhasish-Behera · 2026-02-20T09:09:58Z

Note: Dora isn't showing the status correctly for gloas but using curl or kurtosis logs would be fine.

seems like it will work with after the support of new beacon api additions

Subhasish-Behera · 2026-02-23T08:11:36Z

Identified some additional problems with attestation packing, which is currently not resolved correctly in the PR(current PR used execution state for external attestsions which is wrong. so need a structure to store pre-pool attestation's payload_status to be later used)- worked for kurtosis live as attestations were same slot attestations. Fix incoming
Edit: Updated the PR with the possible fix.

Subhasish-Behera force-pushed the Subh/gloas_fc_changes branch from 7740b80 to 9723120 Compare December 2, 2025 07:03

hangleang added the epbs label Dec 5, 2025

povi reviewed Dec 12, 2025

View reviewed changes

Subhasish-Behera force-pushed the Subh/gloas_fc_changes branch from 9723120 to bb88d6f Compare January 15, 2026 06:56

Subhasish-Behera force-pushed the Subh/gloas_fc_changes branch from bb88d6f to 8cad66e Compare January 29, 2026 11:42

Subhasish-Behera requested a review from povi February 5, 2026 08:11