Skip to content

Fix slot check in proposer preferences gossip - #5191

Merged
jtraglia merged 1 commit into
ethereum:masterfrom
nflaig:fix-slot-check
Apr 29, 2026
Merged

Fix slot check in proposer preferences gossip#5191
jtraglia merged 1 commit into
ethereum:masterfrom
nflaig:fix-slot-check

Conversation

@nflaig

@nflaig nflaig commented Apr 29, 2026

Copy link
Copy Markdown
Member

We wanna check if proposal_slot is greater than the current_slot, ie. not in the past, in #5190 this check was mistakenly inverted.

@jtraglia jtraglia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops. The wording here was confusing & it was previously correct.

@jtraglia
jtraglia merged commit df7a4fc into ethereum:master Apr 29, 2026
15 checks passed
pull Bot pushed a commit to Hawthorne001/prysm that referenced this pull request May 13, 2026
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

The proposer preference cache was keyed by slot only
(`map[Slot]ProposerPreference`) with first-write-wins semantics. During
a reorg that changes the proposer shuffling (different RANDAO at epoch
boundary), a new proposer's preferences for the same slot would be
rejected because the slot already had an entry. This caused:
- Gossip dedup blocking the new proposer's preferences
- Bid validation using the wrong validator's fee recipient and gas limit
- Payload attributes using the wrong fee recipient

Makes the proposer preferences cache reorg-safe by re-keying from `slot`
to `(slot, validatorIndex)`. After a reorg that changes the proposer
shuffling, the correct proposer's preferences are resolved dynamically
from the head state's proposer lookahead.

## Reproducing reorg testing with Kurtosis

To validate this change under real reorgs, add a temporary broadcast
delay flag that is NOT part of this PR. Apply the following patch
locally, build new images, and run with the kurtosis config below.

### 1. Patch `config/features/flags.go`

Add a flag variable before `DisableDutiesV2`:

```go
reorgTestBroadcastDelay = &cli.DurationFlag{
    Name:  "reorg-test-broadcast-delay",
    Usage: "(Testing): Delays P2P block broadcast by this duration while processing the block locally first.",
}
```

Add `reorgTestBroadcastDelay,` to the `BeaconChainFlags` slice.

### 2. Patch `config/features/config.go`

Add a field to the `Flags` struct:

```go
ReorgTestBroadcastDelay time.Duration
```

Add to `ConfigureBeaconChain()` before `Init(cfg)`:

```go
if ctx.IsSet(reorgTestBroadcastDelay.Name) {
    cfg.ReorgTestBroadcastDelay = ctx.Duration(reorgTestBroadcastDelay.Name)
}
```

### 3. Patch `beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go`

Add `"github.com/OffchainLabs/prysm/v7/config/features"` to imports.

Replace `broadcastReceiveBlock` with:

```go
func (vs *Server) broadcastReceiveBlock(ctx context.Context, wg *sync.WaitGroup, block interfaces.SignedBeaconBlock, root [fieldparams.RootLength]byte) error {
    delay := features.Get().ReorgTestBroadcastDelay
    if delay > 0 {
        // Receive locally first, then delay broadcast — creates a divergent fork.
        vs.BlockNotifier.BlockFeed().Send(&feed.Event{
            Type: blockfeed.ReceivedBlock,
            Data: &blockfeed.ReceivedBlockData{SignedBlock: block},
        })
        if err := vs.BlockReceiver.ReceiveBlock(ctx, block, root, nil); err != nil {
            return errors.Wrap(err, "receive block")
        }
        time.Sleep(delay)
        if err := vs.broadcastBlock(ctx, wg, block, root); err != nil {
            return errors.Wrap(err, "broadcast block")
        }
        return nil
    }

    if err := vs.broadcastBlock(ctx, wg, block, root); err != nil {
        return errors.Wrap(err, "broadcast block")
    }
    vs.BlockNotifier.BlockFeed().Send(&feed.Event{
        Type: blockfeed.ReceivedBlock,
        Data: &blockfeed.ReceivedBlockData{SignedBlock: block},
    })
    if err := vs.BlockReceiver.ReceiveBlock(ctx, block, root, nil); err != nil {
        return errors.Wrap(err, "receive block")
    }
    return nil
}
```

### 4. Kurtosis config (`gloas-config-4node-reorg.yml`)

```yaml
participants:
  el:
    el_type: geth
    el_image: ethpandaops/geth:glamsterdam-devnet-0
  cl:
    cl_type: prysm
    cl_image: gcr.io/offchainlabs/prysm/beacon-chain:latest
    vc_image: gcr.io/offchainlabs/prysm/validator:latest
    supernode: true
    count: 2
    vc_extra_params:
      - "--verbosity=debug"
  - el_type: geth
    el_image: ethpandaops/geth:epbs-devnet-0
    cl_type: prysm
    cl_image: gcr.io/offchainlabs/prysm/beacon-chain:latest
    vc_image: gcr.io/offchainlabs/prysm/validator:latest
    supernode: true
    count: 2
    cl_extra_params:
      - "--reorg-test-broadcast-delay=5s"
    vc_extra_params:
      - "--verbosity=debug"

network_params:
  fulu_fork_epoch: 0
  gloas_fork_epoch: 2
  seconds_per_slot: 4
  genesis_delay: 40

additional_services:
  - dora

global_log_level: debug

dora_params:
  image: ethpandaops/dora:gloas-support
```

### 5. Run and monitor

```bash
kurtosis run --enclave gloas-reorg github.com/ethpandaops/ethereum-package \
  --args-file gloas-config-4node-reorg.yml

# Check for reorgs (delayed nodes experience them)
kurtosis service logs gloas-reorg cl-3-prysm-geth --all 2>&1 | grep "Chain reorg occurred"

# Check proposer preferences are flowing
kurtosis service logs gloas-reorg cl-1-prysm-geth --all 2>&1 | grep "Processed signed proposer"

# Key log: cache accepted a different validator's preference for a slot that already had one.
# This is the reorg-safety signal — proves the (slot, validatorIndex) keying works.
kurtosis service logs gloas-reorg cl-3-prysm-geth --all 2>&1 | grep "possible reorg"
```

With 4-second slots and a 5-second delay, the delayed nodes' blocks
arrive after the next slot begins, reliably triggering reorgs (depth
2-3).

### Logs to look for

| Log message | Source | Meaning |
|---|---|---|
| `Chain reorg occurred` | `blockchain` | A reorg happened on this node
|
| `New proposer preference for slot that already has a different
validator (possible reorg)` | `cache` | The cache accepted a second
validator's preference for the same slot — proves the reorg-safe keying
works |
| `Processed signed proposer preferences` | `rpc/validator` | VC
submitted preferences via RPC (shows broadcast/duplicate/total counts) |

**Which issues(s) does this PR fix?**

Fixes #OffchainLabs#16616

related specs 
ethereum/consensus-specs#5196
ethereum/consensus-specs#5190
ethereum/consensus-specs#5191

**Other notes for review**

**Acknowledgements**

- [x] I have read
[CONTRIBUTING.md](https://github.com/prysmaticlabs/prysm/blob/develop/CONTRIBUTING.md).
- [x] I have included a uniquely named [changelog fragment
file](https://github.com/prysmaticlabs/prysm/blob/develop/CONTRIBUTING.md#maintaining-changelogmd).
- [x] I have added a description with sufficient context for reviewers
to understand this PR.
- [x] I have tested that my changes work as expected and I added a
testing plan to the PR description (if applicable).

---------

Co-authored-by: terence <terence@prysmaticlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants