Implement EIP7805: Fork-choice enforced Inclusion Lists #14754

terencechain · 2024-12-28T17:24:55Z

Initial implementation of FOCIL based on:

EIP: https://eips.ethereum.org/EIPS/eip-7805
Consensus specs: Add EIP-7805 (FOCIL) specs ethereum/consensus-specs#4003

Looking for feedback on the spec, design decisions, and end to end testing strategies

jihoonsong · 2024-12-30T08:04:59Z

proto/prysm/v1alpha1/focil.proto

+    uint64 slot = 1 [(ethereum.eth.ext.cast_type) = "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives.Slot"];
+    uint64 validator_index = 2 [(ethereum.eth.ext.cast_type) = "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives.ValidatorIndex"];
+    bytes inclusion_list_committee_root = 3 [(ethereum.eth.ext.ssz_size) = "32"];
+    repeated bytes transactions = 4 [(ethereum.eth.ext.ssz_size) = "?,?", (ethereum.eth.ext.ssz_max)  = "1048576,1073741824"];


Will allowing max size of transactions to 1 GiB instead of 8 KiB leave the room for DoS attack?

No, this is a ssz list size, it's just a theoretical limit. The 8kb check is enforced on the gossip level

jihoonsong · 2024-12-30T08:06:16Z

beacon-chain/sync/inclusion_list.go

+		totalSize += len(transaction)
+	}
+	if totalSize > 8*1024 {
+		return pubsub.ValidationReject, errors.New("total size of transactions exceeds 8KB")


Suggested change

return pubsub.ValidationReject, errors.New("total size of transactions exceeds 8KB")

return pubsub.ValidationReject, errors.New("total size of transactions exceeds 8KiB")

jihoonsong · 2024-12-30T08:48:58Z

beacon-chain/core/helpers/inclusion_list.go

+// GetInclusionListCommittee retrieves the validator indices assigned to the inclusion list committee
+// for a given slot. Returns an error if the state or slot does not meet the required constraints.
+func GetInclusionListCommittee(ctx context.Context, state state.ReadOnlyBeaconState, slot primitives.Slot) ([]primitives.ValidatorIndex, error) {
+	if state.Version() < version.Electra {


Suggested change

if state.Version() < version.Electra {

if state.Version() < version.FOCIL {

jihoonsong · 2024-12-30T09:12:57Z

validator/client/validator.go

@@ -743,6 +743,10 @@ func (v *validator) RolesAt(ctx context.Context, slot primitives.Slot) (map[[fie
 			syncCommitteeValidators[duty.ValidatorIndex] = bytesutil.ToBytes48(duty.PublicKey)
 		}

+		if duty.InclusionListCommitteeSlot == slot {


I guess this is a known issue but beacon REST API for UpdateDuties() is not updated. It could save us some time in the future to explicitly add a TODO in dutiesForEpoch() in validator/client/beacon-api/duties.go.

jihoonsong · 2024-12-30T09:14:23Z

beacon-chain/core/helpers/inclusion_list.go

+
+// GetInclusionListCommittee retrieves the validator indices assigned to the inclusion list committee
+// for a given slot. Returns an error if the state or slot does not meet the required constraints.
+func GetInclusionListCommittee(ctx context.Context, state state.ReadOnlyBeaconState, slot primitives.Slot) ([]primitives.ValidatorIndex, error) {


This one gets called multiple times. What do you think about caching it?

I think it makes sense to do it later. I don't want to introduce premature optimization unless it's necessary at this point. It's not a blocker to interop

jihoonsong · 2024-12-30T13:43:10Z

beacon-chain/execution/engine_client_focil.go

+	defer cancel()
+
+	result := &primitives.PayloadID{}
+	err := s.rpcClient.CallContext(ctx, result, UpdatePayloadWithInclusionListV1, &InclusionListV1{


Suggested change

err := s.rpcClient.CallContext(ctx, result, UpdatePayloadWithInclusionListV1, &InclusionListV1{

err := s.rpcClient.CallContext(ctx, result, UpdatePayloadWithInclusionListV1, payloadID, &InclusionListV1{

jihoonsong · 2024-12-30T14:02:00Z

beacon-chain/sync/inclusion_list.go

+	// Validate slot constraints.
+	currentSlot := s.cfg.clock.CurrentSlot()
+	if il.Message.Slot == currentSlot || il.Message.Slot+1 == currentSlot {
+		return pubsub.ValidationReject, errors.New("slot is equal to the previous or current slot")


Suggested change

return pubsub.ValidationReject, errors.New("slot is equal to the previous or current slot")

return pubsub.ValidationReject, errors.New("slot is not equal to the previous or current slot")

jihoonsong · 2024-12-30T14:05:45Z

beacon-chain/sync/inclusion_list.go

+		return pubsub.ValidationIgnore, err
+	}
+	deadline := params.BeaconConfig().SecondsPerSlot / params.BeaconConfig().IntervalsPerSlot
+	if il.Message.Slot+1 == currentSlot && secondsSinceSlotStart > deadline {


Should we need to change consensus spec's p2p part to [IGNORE] The slot message.slot is equal to the previous slot and the current time is less than attestation_deadline seconds into the slot.?

Yes we should

jihoonsong · 2024-12-30T14:06:24Z

beacon-chain/sync/inclusion_list.go

+	}
+	deadline := params.BeaconConfig().SecondsPerSlot / params.BeaconConfig().IntervalsPerSlot
+	if il.Message.Slot+1 == currentSlot && secondsSinceSlotStart > deadline {
+		return pubsub.ValidationIgnore, errors.New("slot is equal to the current slot, or it is equal to the previous slot and the current time is less than attestation_deadline seconds into the slot")


It seems the error messages here show why it went wrong instead of what it should be.

jihoonsong · 2024-12-30T14:16:20Z

beacon-chain/blockchain/service.go

@@ -215,6 +217,7 @@ func (s *Service) Start() {
 	}
 	s.spawnProcessAttestationsRoutine()
 	go s.runLateBlockTasks()
+	go s.updateBlockWithInclusionListRoutine()


What makes it better to have a routine here instead of updating a payload when a validator has a proposer role? Easier implementation?

The routine is a no op unless the validator has a proposer role regardless

jihoonsong · 2024-12-31T07:35:24Z

beacon-chain/cache/inclusion_list.go

+	if entry.txs == nil {
+		entry.txs = txs
+	} else {
+		entry.seenTwice = true


Is there any chance the same IL received from different peer?

Libp2p covers such case, it filters by content so we should not be importing same IL from different peer

terencechain requested a review from a team as a code owner December 28, 2024 17:24

terencechain requested review from kasey, rkapka and dB2510 December 28, 2024 17:24

terencechain force-pushed the focil branch from 05178e5 to 2671fcf Compare December 28, 2024 17:35

jihoonsong reviewed Dec 30, 2024

View reviewed changes

terencechain force-pushed the focil branch 4 times, most recently from 23b7573 to 7536720 Compare December 31, 2024 03:51

jihoonsong reviewed Dec 31, 2024

View reviewed changes

terencechain force-pushed the focil branch from 7536720 to 968bc95 Compare January 2, 2025 05:44

Implement EIP7805: Fork-choice enforced Inclusion Lists

2f30f2a

terencechain force-pushed the focil branch from 968bc95 to 2f30f2a Compare January 2, 2025 05:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement EIP7805: Fork-choice enforced Inclusion Lists #14754

Implement EIP7805: Fork-choice enforced Inclusion Lists #14754

terencechain commented Dec 28, 2024

jihoonsong Dec 30, 2024

terencechain Dec 30, 2024

jihoonsong Dec 30, 2024

jihoonsong Dec 30, 2024

jihoonsong Dec 30, 2024

jihoonsong Dec 30, 2024

terencechain Dec 30, 2024

jihoonsong Dec 30, 2024

jihoonsong Dec 30, 2024

jihoonsong Dec 30, 2024

terencechain Dec 30, 2024

jihoonsong Dec 30, 2024

jihoonsong Dec 30, 2024

terencechain Dec 30, 2024

jihoonsong Dec 31, 2024

terencechain Dec 31, 2024 •

edited

Loading

	return pubsub.ValidationReject, errors.New("total size of transactions exceeds 8KB")
	return pubsub.ValidationReject, errors.New("total size of transactions exceeds 8KiB")

	if state.Version() < version.Electra {
	if state.Version() < version.FOCIL {

	err := s.rpcClient.CallContext(ctx, result, UpdatePayloadWithInclusionListV1, &InclusionListV1{
	err := s.rpcClient.CallContext(ctx, result, UpdatePayloadWithInclusionListV1, payloadID, &InclusionListV1{

Implement EIP7805: Fork-choice enforced Inclusion Lists #14754

Are you sure you want to change the base?

Implement EIP7805: Fork-choice enforced Inclusion Lists #14754

Conversation

terencechain commented Dec 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terencechain Dec 31, 2024 • edited Loading

Choose a reason for hiding this comment

terencechain Dec 31, 2024 •

edited

Loading