Skip to content

Commit 3405e17

Browse files
committed
fix(fork-choice): port 3 upstream fixes from lodestar unstable
- Initialize anchor block PTC votes to all-true per spec get_forkchoice_store - Reject same-slot full attestation votes and require FULL variant for index=1 - Add hasPayload to proto_array/fork_choice, refactor isPayloadTimely to use it
1 parent 9ee58ac commit 3405e17

File tree

2 files changed

+204
-8
lines changed

2 files changed

+204
-8
lines changed

src/fork_choice/fork_choice.zig

Lines changed: 148 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ pub const ForkChoiceError = ProtoArrayError || error{
7777
InvalidAttestationAttestsToFutureBlock,
7878
InvalidAttestationFutureSlot,
7979
InvalidAttestationInvalidDataIndex,
80+
InvalidAttestationUnknownPayloadStatus,
8081
// InvalidBlock inner codes
8182
InvalidBlockUnknownParent,
8283
InvalidBlockFutureSlot,
@@ -808,9 +809,18 @@ pub const ForkChoice = struct {
808809
// the case, the attestation should not be considered.
809810
if (block.slot > slot) return error.InvalidAttestationAttestsToFutureBlock;
810811

811-
// INVALID_DATA_INDEX: For Gloas blocks, attestation index must be 0 or 1.
812+
// Gloas attestation validation.
812813
const att_index = attestation.index();
813-
if (block.isGloasBlock() and att_index != 0 and att_index != 1) return error.InvalidAttestationInvalidDataIndex;
814+
if (block.isGloasBlock()) {
815+
// INVALID_DATA_INDEX: For Gloas blocks, attestation index must be 0 or 1.
816+
if (att_index != 0 and att_index != 1) return error.InvalidAttestationInvalidDataIndex;
817+
818+
// Same-slot attestations can only vote for the PENDING variant (index 0).
819+
if (block.slot == slot and att_index != 0) return error.InvalidAttestationInvalidDataIndex;
820+
821+
// Voting for FULL (index=1) requires the FULL variant to exist.
822+
if (att_index == 1 and !self.proto_array.hasPayload(block_root)) return error.InvalidAttestationUnknownPayloadStatus;
823+
}
814824

815825
// Cache validated attestation data root.
816826
try self.validated_attestation_datas.put(
@@ -1574,6 +1584,12 @@ pub const ForkChoice = struct {
15741584
return self.proto_array.hasBlock(block_root);
15751585
}
15761586

1587+
/// Returns true if the FULL payload variant (execution payload envelope) exists for this
1588+
/// block root, without checking finalized-descendant status.
1589+
pub fn hasPayloadUnsafe(self: *const ForkChoice, block_root: Root) bool {
1590+
return self.proto_array.hasPayload(block_root);
1591+
}
1592+
15771593
/// Returns a `ProtoBlock` if the block is known **and** a descendant of the finalized root.
15781594
pub fn getBlock(self: *const ForkChoice, block_root: Root, payload_status: PayloadStatus) ?ProtoBlock {
15791595
const node = self.proto_array.getNode(block_root, payload_status) orelse return null;
@@ -4140,3 +4156,133 @@ test "onAttestation: equivocating validator votes are not counted" {
41404156
try fc.updateHead(allocator);
41414157
try testing.expectEqual(b_root, fc.head.block_root);
41424158
}
4159+
4160+
test "onAttestation: reject same-slot full vote for Gloas block" {
4161+
const allocator = testing.allocator;
4162+
const genesis_root = hashFromByte(0x01);
4163+
const block_root = hashFromByte(0x02);
4164+
const genesis_block = makeTestBlock(0, genesis_root, ZERO_HASH);
4165+
const balances = [_]u16{100} ** 2;
4166+
4167+
var fc = try initTestForkChoice(
4168+
allocator,
4169+
genesis_block,
4170+
10,
4171+
makeTestCheckpoint(0, genesis_root),
4172+
makeTestCheckpoint(0, genesis_root),
4173+
&balances,
4174+
);
4175+
defer deinitTestForkChoice(allocator, fc);
4176+
4177+
// Add a Gloas block at slot 1.
4178+
try onBlockFromProto(fc, allocator, makeGloasTestBlock(1, block_root, genesis_root, ZERO_HASH), 10);
4179+
4180+
// Same-slot attestation (slot=1) with index=1 (FULL vote) must be rejected.
4181+
const indices = [_]ValidatorIndex{0};
4182+
var att = makeTestIndexedAttestation(&indices, 1, block_root, 0, block_root, 0, genesis_root, 1);
4183+
const any_att = AnyIndexedAttestation{ .phase0 = &att };
4184+
try testing.expectError(
4185+
error.InvalidAttestationInvalidDataIndex,
4186+
fc.onAttestation(allocator, &any_att, hashFromByte(0xB1), false),
4187+
);
4188+
}
4189+
4190+
test "onAttestation: reject full vote when FULL variant missing" {
4191+
const allocator = testing.allocator;
4192+
const genesis_root = hashFromByte(0x01);
4193+
const block_root = hashFromByte(0x02);
4194+
const genesis_block = makeTestBlock(0, genesis_root, ZERO_HASH);
4195+
const balances = [_]u16{100} ** 2;
4196+
4197+
var fc = try initTestForkChoice(
4198+
allocator,
4199+
genesis_block,
4200+
10,
4201+
makeTestCheckpoint(0, genesis_root),
4202+
makeTestCheckpoint(0, genesis_root),
4203+
&balances,
4204+
);
4205+
defer deinitTestForkChoice(allocator, fc);
4206+
4207+
// Add a Gloas block at slot 1 (only PENDING + EMPTY, no FULL).
4208+
try onBlockFromProto(fc, allocator, makeGloasTestBlock(1, block_root, genesis_root, ZERO_HASH), 10);
4209+
4210+
// Attestation from a later slot (slot=2) with index=1 (FULL vote)
4211+
// must be rejected because no FULL variant exists yet.
4212+
const indices = [_]ValidatorIndex{0};
4213+
var att = makeTestIndexedAttestation(&indices, 2, block_root, 0, block_root, 0, genesis_root, 1);
4214+
const any_att = AnyIndexedAttestation{ .phase0 = &att };
4215+
try testing.expectError(
4216+
error.InvalidAttestationUnknownPayloadStatus,
4217+
fc.onAttestation(allocator, &any_att, hashFromByte(0xB2), false),
4218+
);
4219+
}
4220+
4221+
test "hasPayloadUnsafe reflects onExecutionPayload" {
4222+
const allocator = testing.allocator;
4223+
const genesis_root = hashFromByte(0x01);
4224+
const block_root = hashFromByte(0x02);
4225+
const genesis_block = makeTestBlock(0, genesis_root, ZERO_HASH);
4226+
const balances = [_]u16{100} ** 2;
4227+
4228+
var fc = try initTestForkChoice(
4229+
allocator,
4230+
genesis_block,
4231+
10,
4232+
makeTestCheckpoint(0, genesis_root),
4233+
makeTestCheckpoint(0, genesis_root),
4234+
&balances,
4235+
);
4236+
defer deinitTestForkChoice(allocator, fc);
4237+
4238+
// Pre-Gloas genesis always has payload.
4239+
try testing.expect(fc.hasPayloadUnsafe(genesis_root));
4240+
4241+
// Add Gloas block — no FULL yet.
4242+
try onBlockFromProto(fc, allocator, makeGloasTestBlock(1, block_root, genesis_root, ZERO_HASH), 10);
4243+
try testing.expect(!fc.hasPayloadUnsafe(block_root));
4244+
4245+
// After execution payload arrives, FULL exists.
4246+
try fc.onExecutionPayload(allocator, block_root, hashFromByte(0xEE), 1, ZERO_HASH, .valid);
4247+
try testing.expect(fc.hasPayloadUnsafe(block_root));
4248+
}
4249+
4250+
test "getCanonicalBlockByRoot finds ancestor on canonical chain" {
4251+
const allocator = testing.allocator;
4252+
const genesis_root = hashFromByte(0x01);
4253+
const a_root = hashFromByte(0x0A);
4254+
const b_root = hashFromByte(0x0B);
4255+
const genesis_block = makeTestBlock(0, genesis_root, ZERO_HASH);
4256+
const balances = [_]u16{100} ** 2;
4257+
4258+
var fc = try initTestForkChoice(
4259+
allocator,
4260+
genesis_block,
4261+
10,
4262+
makeTestCheckpoint(0, genesis_root),
4263+
makeTestCheckpoint(0, genesis_root),
4264+
&balances,
4265+
);
4266+
defer deinitTestForkChoice(allocator, fc);
4267+
4268+
try onBlockFromProto(fc, allocator, makeTestBlock(1, a_root, genesis_root), 10);
4269+
try onBlockFromProto(fc, allocator, makeTestBlock(2, b_root, a_root), 10);
4270+
try fc.updateHead(allocator);
4271+
4272+
// Head should be B (longest chain).
4273+
try testing.expectEqual(b_root, fc.head.block_root);
4274+
4275+
// A is on the canonical chain.
4276+
const found_a = try fc.getCanonicalBlockByRoot(a_root);
4277+
try testing.expect(found_a != null);
4278+
try testing.expectEqual(a_root, found_a.?.block_root);
4279+
4280+
// Head itself is found.
4281+
const found_b = try fc.getCanonicalBlockByRoot(b_root);
4282+
try testing.expect(found_b != null);
4283+
try testing.expectEqual(b_root, found_b.?.block_root);
4284+
4285+
// Non-existent root returns null.
4286+
const not_found = try fc.getCanonicalBlockByRoot(hashFromByte(0xFF));
4287+
try testing.expect(not_found == null);
4288+
}

src/fork_choice/proto_array.zig

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,13 @@ pub const ProtoArray = struct {
561561
anchor.target_root = block.block_root;
562562

563563
try self.onBlock(allocator, anchor, current_slot, null);
564+
565+
// Anchor block PTC votes must be all-true per spec get_forkchoice_store:
566+
// payload_timeliness_vote={anchor_root: Vector[boolean, PTC_SIZE](True for _ in range(PTC_SIZE))}
567+
// Spec: https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.4/specs/gloas/fork-choice.md#modified-get_forkchoice_store
568+
if (self.ptc_votes.getPtr(block.block_root)) |votes| {
569+
votes.* = PtcVotes.initFull();
570+
}
564571
}
565572

566573
// ── Accessors ──
@@ -600,6 +607,16 @@ pub const ProtoArray = struct {
600607
return self.indices.contains(root);
601608
}
602609

610+
/// Returns true if the FULL payload variant exists for this block root.
611+
/// This means the SignedExecutionPayloadEnvelope has been received and processed.
612+
pub fn hasPayload(self: *const ProtoArray, root: Root) bool {
613+
const vi = self.indices.get(root) orelse return false;
614+
return switch (vi) {
615+
.pre_gloas => true, // Pre-Gloas blocks always have their payload.
616+
.gloas => |g| g.full != null,
617+
};
618+
}
619+
603620
// ── onBlock ──
604621

605622
/// Register a block with the fork choice. It is only sane to supply
@@ -1710,12 +1727,8 @@ pub const ProtoArray = struct {
17101727
// Block not found or not a Gloas block.
17111728
const votes = self.ptc_votes.get(block_root) orelse return false;
17121729

1713-
// Payload is locally available if proto array
1714-
// has FULL variant of the block.
1715-
if (self.getNodeIndexByRootAndStatus(
1716-
block_root,
1717-
.full,
1718-
) == null) return false;
1730+
// Payload is locally available if FULL variant exists.
1731+
if (!self.hasPayload(block_root)) return false;
17191732

17201733
// Count votes for payload_present=true.
17211734
return votes.count() > PAYLOAD_TIMELY_THRESHOLD;
@@ -5171,3 +5184,40 @@ test "invalid node not re-validated by valid LVH" {
51715184
// B must remain invalid — the valid response must NOT resurrect it.
51725185
try testing.expectEqual(ExecutionStatus.invalid, pa.nodes.items[idx_b].extra_meta.executionStatus());
51735186
}
5187+
5188+
test "initialize sets Gloas anchor PTC votes to all-true" {
5189+
var pa: ProtoArray = undefined;
5190+
pa.init(0, ZERO_HASH, 0, ZERO_HASH, 0);
5191+
defer pa.deinit(testing.allocator);
5192+
5193+
const root = makeRoot(1);
5194+
const anchor = TestBlock.asGloas(TestBlock.withRoot(root));
5195+
try pa.initialize(testing.allocator, anchor, 0);
5196+
5197+
// Anchor block PTC votes must be all-true per spec get_forkchoice_store.
5198+
const votes = pa.ptc_votes.get(root).?;
5199+
try testing.expectEqual(preset.PTC_SIZE, votes.count());
5200+
}
5201+
5202+
test "hasPayload returns false before and true after onExecutionPayload" {
5203+
var pa: ProtoArray = undefined;
5204+
pa.init(0, ZERO_HASH, 0, ZERO_HASH, 0);
5205+
defer pa.deinit(testing.allocator);
5206+
5207+
// Pre-Gloas block always has payload.
5208+
const pre_gloas_root = makeRoot(0xAA);
5209+
try pa.onBlock(testing.allocator, TestBlock.genesis(), 0, null);
5210+
try testing.expect(pa.hasPayload(ZERO_HASH));
5211+
5212+
// Gloas block: no FULL variant yet.
5213+
const root = makeRoot(1);
5214+
try pa.onBlock(testing.allocator, TestBlock.asGloas(TestBlock.withRoot(root)), 0, null);
5215+
try testing.expect(!pa.hasPayload(root));
5216+
5217+
// After onExecutionPayload, FULL variant exists.
5218+
try pa.onExecutionPayload(testing.allocator, root, 0, makeRoot(0xBB), 42, ZERO_HASH, null, .valid);
5219+
try testing.expect(pa.hasPayload(root));
5220+
5221+
// Unknown root returns false.
5222+
try testing.expect(!pa.hasPayload(pre_gloas_root));
5223+
}

0 commit comments

Comments
 (0)