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
2 changes: 1 addition & 1 deletion packages/beacon-node/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,7 @@ export function getValidatorApi(
// It should be set to the latest block applied to `self` or the genesis block root.
const dependentRoot =
// In v2 the dependent root is different after fulu due to deterministic proposer lookahead
proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state, epoch) ||
proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state) ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve the v2 next-epoch dependent root

When v2 post-Fulu duties for epoch N+1 are served from a state at epoch N, this call now derives the root from state.epoch and returns block_root(start(N-1)-1). Those duties come from state.nextProposers, which is keyed by state.nextDecisionRoot / the v2 spec root block_root(start(N)-1); the Gloas proposer-preference validator also checks headState.nextDecisionRoot for proposalEpoch - 1. As a result, validator clients that poll next-epoch v2 duties will sign preferences with a dependent root the beacon node rejects as unknown.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed — closing #9498 in favor of #9499 which keeps proposalEpoch as a parameter and adopts the Lighthouse-style head-root fallback for the future-decision-slot case. That preserves the v2 post-Fulu block_root(start(N) - 1) semantics you flagged here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

By removing epoch from proposerShufflingDecisionRoot and relying solely on state.epoch, we introduce a bug when requesting proposer duties for the previous epoch (currentEpoch - 1). In that case, state.epoch is currentEpoch, so the returned dependent root will incorrectly correspond to the current epoch instead of the previous epoch. We should keep passing epoch here and handle the mid-epoch next-epoch future slot issue inside proposerShufflingDecisionRoot by capping the decision slot.

Suggested change
proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state) ||
proposerShufflingDecisionRoot(opts?.v2 ? config.getForkName(startSlot) : ForkName.phase0, state, epoch) ||

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed — closing #9498. The prev-epoch breakage you flagged is real, and state.epoch was the wrong anchor. #9499 keeps proposalEpoch as a parameter.

(await getGenesisBlockRoot(state));

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {afterEach, beforeEach, describe, expect, it, vi} from "vitest";
import {routes} from "@lodestar/api";
import {getConfig} from "@lodestar/config/test-utils";
import {ForkName, MAX_EFFECTIVE_BALANCE, SLOTS_PER_EPOCH} from "@lodestar/params";
import {BeaconStateAllForks, BeaconStateView} from "@lodestar/state-transition";
import {BeaconStateAllForks, BeaconStateView, computeStartSlotAtEpoch} from "@lodestar/state-transition";
import {Slot} from "@lodestar/types";
import {SYNC_TOLERANCE_EPOCHS, getValidatorApi} from "../../../../../../src/api/impl/validator/index.js";
import {defaultApiOptions} from "../../../../../../src/api/options.js";
Expand Down Expand Up @@ -61,6 +61,7 @@ describe("get proposers api impl", () => {
}

afterEach(() => {
vi.restoreAllMocks();
vi.useRealTimers();
});

Expand Down Expand Up @@ -107,6 +108,22 @@ describe("get proposers api impl", () => {
);
});

it("should get v1 proposers for next epoch mid-epoch", async () => {
const nextEpoch = currentEpoch + 1;
const getBlockRootAtSlotSpy = vi.spyOn(BeaconStateView.prototype, "getBlockRootAtSlot");

const {data: result} = (await api.getProposerDuties({epoch: nextEpoch})) as {
data: routes.validator.ProposerDutyList;
};

expect(result.length).toBe(SLOTS_PER_EPOCH);
expect(getBlockRootAtSlotSpy).toHaveBeenCalledWith(currentSlot - 1);
expect(getBlockRootAtSlotSpy).not.toHaveBeenCalledWith(computeStartSlotAtEpoch(nextEpoch) - 1);
expect(result.map((p) => p.slot)).toEqual(
Array.from({length: SLOTS_PER_EPOCH}, (_, i) => nextEpoch * SLOTS_PER_EPOCH + i)
);
});

it("should get proposers for historical epoch", async () => {
const historicalEpoch = currentEpoch - 2;
initializeState(currentSlot - 2 * SLOTS_PER_EPOCH + 1);
Expand Down
20 changes: 7 additions & 13 deletions packages/state-transition/src/util/shuffling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,24 @@ import {computeStartSlotAtEpoch} from "./epoch.js";
import {EpochShuffling} from "./epochShuffling.js";

/**
* Block root that decided the proposer shuffling for `proposalEpoch` (keys that shuffling).
* Computed for the requested epoch, not `state.epoch`, so it is correct when `state` is one
* epoch off the requested epoch (e.g. serving next-epoch duties from the current state).
* Block root that decided the proposer shuffling for the state used to serve proposer duties.
*
* Returns `null` when the genesis block decides its own shuffling (caller falls back to the
* genesis block root).
*/
export function proposerShufflingDecisionRoot(
fork: ForkName,
state: IBeaconStateView,
proposalEpoch: Epoch
): Root | null {
const decisionSlot = proposerShufflingDecisionSlot(fork, proposalEpoch);
export function proposerShufflingDecisionRoot(fork: ForkName, state: IBeaconStateView): Root | null {
const decisionSlot = proposerShufflingDecisionSlot(fork, state);
if (state.slot === decisionSlot) {
return null;
}
return state.getBlockRootAtSlot(decisionSlot);
}

/** Slot whose block root keys the proposer shuffling for `proposalEpoch`. */
function proposerShufflingDecisionSlot(fork: ForkName, proposalEpoch: Epoch): Slot {
/** Slot whose block root keys the proposer shuffling for `state`. */
function proposerShufflingDecisionSlot(fork: ForkName, state: IBeaconStateView): Slot {
// Post-Fulu the shuffling is decided one epoch earlier (deterministic proposer lookahead,
// MIN_SEED_LOOKAHEAD = 1); pre-Fulu it is the last block before `proposalEpoch`.
const decisionEpoch = isForkPostFulu(fork) ? proposalEpoch - 1 : proposalEpoch;
// MIN_SEED_LOOKAHEAD = 1); pre-Fulu it is the last block before `state.epoch`.
const decisionEpoch = isForkPostFulu(fork) ? state.epoch - 1 : state.epoch;
return Math.max(computeStartSlotAtEpoch(decisionEpoch) - 1, 0);
}
Comment on lines +20 to 39

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

To prevent returning the wrong dependent root for previous epoch requests (where state.epoch is currentEpoch but the requested epoch is currentEpoch - 1), we should keep proposalEpoch in the signature. To resolve the original issue where requesting the next epoch mid-epoch pre-Fulu attempts to read a future block root, we can cap the computed decisionSlot to the last slot of the state's current epoch if it is in the future.

 * Block root that decided the proposer shuffling for `proposalEpoch` (keys that shuffling).
 * Computed for the requested epoch, not `state.epoch`, so it is correct when `state` is one
 * epoch off the requested epoch (e.g. serving next-epoch duties from the current state).
 *
 * Returns `null` when the genesis block decides its own shuffling (caller falls back to the
 * genesis block root).
 */
export function proposerShufflingDecisionRoot(
  fork: ForkName,
  state: IBeaconStateView,
  proposalEpoch: Epoch
): Root | null {
  const decisionSlot = proposerShufflingDecisionSlot(fork, proposalEpoch, state.slot);
  if (state.slot === decisionSlot) {
    return null;
  }
  return state.getBlockRootAtSlot(decisionSlot);
}

/** Slot whose block root keys the proposer shuffling for `proposalEpoch`. */
function proposerShufflingDecisionSlot(fork: ForkName, proposalEpoch: Epoch, currentSlot: Slot): Slot {
  // Post-Fulu the shuffling is decided one epoch earlier (deterministic proposer lookahead,
  // MIN_SEED_LOOKAHEAD = 1); pre-Fulu it is the last block before `proposalEpoch`.
  const decisionEpoch = isForkPostFulu(fork) ? proposalEpoch - 1 : proposalEpoch;
  const decisionSlot = Math.max(computeStartSlotAtEpoch(decisionEpoch) - 1, 0);
  if (decisionSlot >= currentSlot) {
    const currentEpoch = Math.floor(currentSlot / SLOTS_PER_EPOCH);
    return Math.max(computeStartSlotAtEpoch(currentEpoch) - 1, 0);
  }
  return decisionSlot;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Closing #9498#9499 takes the same direction your suggestion pointed at (keep proposalEpoch, handle the future-decision-slot case at the helper boundary) but adopts the Lighthouse-style head-root fallback rather than a cap, matching the beacon-APIs V1 event.block rule for that case. Genesis edge case is preserved via a Root | null return + getGenesisBlockRoot(state) fallback at the call site (per Codex's P2 on #9499).


Expand Down