Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fork-specific consensus-types interfaces #13948

Merged
merged 5 commits into from
May 3, 2024
Merged

Fork-specific consensus-types interfaces #13948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e1620c9
fork-specific interface for electra
kasey Apr 26, 2024
3610879
add electra to wrapped payload switch
kasey May 2, 2024
ff10f90
use electra body in block factory
kasey May 2, 2024
5b2f60f
deepsource
kasey May 2, 2024
d5d6e9e
rm pb getters from electra payload
kasey May 2, 2024
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
158 changes: 32 additions & 126 deletions consensus-types/blocks/execution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ func NewWrappedExecutionData(v proto.Message, weiValue math.Wei) (interfaces.Exe
return WrappedExecutionPayloadCapella(pbStruct, weiValue)
case *enginev1.ExecutionPayloadDeneb:
return WrappedExecutionPayloadDeneb(pbStruct, weiValue)
case *enginev1.ExecutionPayloadElectra:
return WrappedExecutionPayloadElectra(pbStruct, weiValue)
default:
return nil, ErrUnsupportedVersion
}
}

var _ interfaces.ExecutionData = &executionPayload{}

// WrappedExecutionPayload is a constructor which wraps a protobuf execution payload into an interface.
func WrappedExecutionPayload(p *enginev1.ExecutionPayload) (interfaces.ExecutionData, error) {
w := executionPayload{p: p}
Expand Down Expand Up @@ -199,23 +203,15 @@ func (executionPayload) ValueInGwei() (uint64, error) {
return 0, consensus_types.ErrUnsupportedField
}

// DepositReceipts --
func (e executionPayload) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return nil, consensus_types.ErrUnsupportedField
}

// WithdrawalRequests --
func (e executionPayload) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return nil, consensus_types.ErrUnsupportedField
}

// executionPayloadHeader is a convenience wrapper around a blinded beacon block body's execution header data structure
// This wrapper allows us to conform to a common interface so that beacon
// blocks for future forks can also be applied across Prysm without issues.
type executionPayloadHeader struct {
p *enginev1.ExecutionPayloadHeader
}

var _ interfaces.ExecutionData = &executionPayloadHeader{}

// WrappedExecutionPayloadHeader is a constructor which wraps a protobuf execution header into an interface.
func WrappedExecutionPayloadHeader(p *enginev1.ExecutionPayloadHeader) (interfaces.ExecutionData, error) {
w := executionPayloadHeader{p: p}
Expand Down Expand Up @@ -375,16 +371,6 @@ func (executionPayloadHeader) ValueInGwei() (uint64, error) {
return 0, consensus_types.ErrUnsupportedField
}

// DepositReceipts --
func (e executionPayloadHeader) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return nil, consensus_types.ErrUnsupportedField
}

// WithdrawalRequests --
func (e executionPayloadHeader) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PayloadToHeader converts `payload` into execution payload header format.
func PayloadToHeader(payload interfaces.ExecutionData) (*enginev1.ExecutionPayloadHeader, error) {
txs, err := payload.Transactions()
Expand Down Expand Up @@ -422,6 +408,8 @@ type executionPayloadCapella struct {
gweiValue uint64
}

var _ interfaces.ExecutionData = &executionPayloadCapella{}

// WrappedExecutionPayloadCapella is a constructor which wraps a protobuf execution payload into an interface.
func WrappedExecutionPayloadCapella(p *enginev1.ExecutionPayloadCapella, value math.Wei) (interfaces.ExecutionData, error) {
w := executionPayloadCapella{p: p, weiValue: value, gweiValue: uint64(math.WeiToGwei(value))}
Expand Down Expand Up @@ -581,16 +569,6 @@ func (e executionPayloadCapella) ValueInGwei() (uint64, error) {
return e.gweiValue, nil
}

// DepositReceipts --
func (e executionPayloadCapella) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return nil, consensus_types.ErrUnsupportedField
}

// WithdrawalRequests --
func (e executionPayloadCapella) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return nil, consensus_types.ErrUnsupportedField
}

// executionPayloadHeaderCapella is a convenience wrapper around a blinded beacon block body's execution header data structure
// This wrapper allows us to conform to a common interface so that beacon
// blocks for future forks can also be applied across Prysm without issues.
Expand All @@ -600,6 +578,8 @@ type executionPayloadHeaderCapella struct {
gweiValue uint64
}

var _ interfaces.ExecutionData = &executionPayloadHeaderCapella{}

// WrappedExecutionPayloadHeaderCapella is a constructor which wraps a protobuf execution header into an interface.
func WrappedExecutionPayloadHeaderCapella(p *enginev1.ExecutionPayloadHeaderCapella, value math.Wei) (interfaces.ExecutionData, error) {
w := executionPayloadHeaderCapella{p: p, weiValue: value, gweiValue: uint64(math.WeiToGwei(value))}
Expand Down Expand Up @@ -759,16 +739,6 @@ func (e executionPayloadHeaderCapella) ValueInGwei() (uint64, error) {
return e.gweiValue, nil
}

// DepositReceipts --
func (e executionPayloadHeaderCapella) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return nil, consensus_types.ErrUnsupportedField
}

// WithdrawalRequests --
func (e executionPayloadHeaderCapella) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PayloadToHeaderCapella converts `payload` into execution payload header format.
func PayloadToHeaderCapella(payload interfaces.ExecutionData) (*enginev1.ExecutionPayloadHeaderCapella, error) {
txs, err := payload.Transactions()
Expand Down Expand Up @@ -856,7 +826,7 @@ func PayloadToHeaderDeneb(payload interfaces.ExecutionData) (*enginev1.Execution
}

// PayloadToHeaderElectra converts `payload` into execution payload header format.
func PayloadToHeaderElectra(payload interfaces.ExecutionData) (*enginev1.ExecutionPayloadHeaderElectra, error) {
func PayloadToHeaderElectra(payload interfaces.ExecutionDataElectra) (*enginev1.ExecutionPayloadHeaderElectra, error) {
txs, err := payload.Transactions()
if err != nil {
return nil, err
Expand All @@ -882,18 +852,13 @@ func PayloadToHeaderElectra(payload interfaces.ExecutionData) (*enginev1.Executi
return nil, err
}

depositReceipts, err := payload.DepositReceipts()
if err != nil {
return nil, err
}
depositReceipts := payload.DepositReceipts()
depositReceiptsRoot, err := ssz.DepositReceiptSliceRoot(depositReceipts, fieldparams.MaxDepositReceiptsPerPayload)
if err != nil {
return nil, err
}
withdrawalRequests, err := payload.WithdrawalRequests()
if err != nil {
return nil, err
}

withdrawalRequests := payload.WithdrawalRequests()
withdrawalRequestsRoot, err := ssz.WithdrawalRequestSliceRoot(withdrawalRequests, fieldparams.MaxWithdrawalRequestsPerPayload)
if err != nil {
return nil, err
Expand Down Expand Up @@ -980,23 +945,14 @@ func IsEmptyExecutionData(data interfaces.ExecutionData) (bool, error) {
return false, nil
}

drs, err := data.DepositReceipts()
switch {
case errors.Is(err, consensus_types.ErrUnsupportedField):
case err != nil:
return false, err
default:
epe, postElectra := data.(interfaces.ExecutionDataElectra)
if postElectra {
drs := epe.DepositReceipts()
Comment on lines +948 to +950
Copy link
Contributor

Choose a reason for hiding this comment

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

This pattern assumes that every interfaces.ExecutionData in future forks, that also satisfies ExecutionDataElectra will have to be handled in the same way. Say for example that interfaces.ExecutionDataEpbs satisfies the Electra interface, it requires different handling of deposits but it requires the same handling of 7002 withdrawals, etc. If we explicitly change it to not satisfy ExecutionDataElectra and satisfy a different ExectutionDataEpbs then we would have to copy paste the withdrawal handling. If we keep it satisfying ExecutionDataElectra then we would have to deal with deposits with an if-else data.(interfaces.ExecutionDataEpbs) within this block right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy/paste from team chat:

If the ssz is the same, then it's fine for ExecutionDataEpbs to satisfy ExecutionDataElectra. Satisfying the interface only dictates what kind of value are available, it does not dictate how the data should be processed. That should always be based on the slot (or version, which is based on the slot).

SSZ changes in nested types is similar to what we just went through with Attestations. In that case our solution was to move attestations to an interface. A benefit of this kind of composition is that we would only need to implement the logic to deal with the interface in 2 places.If we do want to actually deprecate (remove) a field, then yes one option would be to break the chain of interface composition. Another would be to backport an error to that fields signature. Another would be to allow the getter to return a nil value and rely on the caller to be fork-aware. Another option is to rely less on these big interfaces and move processing functions to accept narrower interfaces. For example, epbs could take:

type EpbsWithdrawalGetter interface {
    GetWithdrawals() *eth.PBSWithdrawal
}

whereas other methods would take:

type WithdrawalsGetter interface {
    GetWithdrawals() []byte
}

and then perform assertions on those instead of the whole big type. A problem in both cases is that now if we want block to have something like an ExecutionData type that the container could use to abstractly carry the payload. This type would have to become /increasingly/ abstract (some kind of ConsensusType interface that would have ssz methods etc, which is asserted downstream to narrower interfaces). Otherwise we would also break the chain of composition in the container (block body).

TL;DR this approach handles all the previous fork changes pretty well, with attestations being the only notable exception. If we see more extreme changes across forks I think the best way to deal with that is probably more abstract interfaces for nested types, and narrow interfaces used by consumers of those fields. We'll need to keep iterating on it to see what feels most maintainable in practice.

if len(drs) != 0 {
return false, nil
}
}

wrs, err := data.WithdrawalRequests()
switch {
case errors.Is(err, consensus_types.ErrUnsupportedField):
case err != nil:
return false, err
default:
wrs := epe.WithdrawalRequests()
if len(wrs) != 0 {
return false, nil
}
Expand All @@ -1014,6 +970,8 @@ type executionPayloadHeaderDeneb struct {
gweiValue uint64
}

var _ interfaces.ExecutionData = &executionPayloadHeaderDeneb{}

// WrappedExecutionPayloadHeaderDeneb is a constructor which wraps a protobuf execution header into an interface.
func WrappedExecutionPayloadHeaderDeneb(p *enginev1.ExecutionPayloadHeaderDeneb, value math.Wei) (interfaces.ExecutionData, error) {
w := executionPayloadHeaderDeneb{p: p, weiValue: value, gweiValue: uint64(math.WeiToGwei(value))}
Expand Down Expand Up @@ -1168,16 +1126,6 @@ func (e executionPayloadHeaderDeneb) ValueInGwei() (uint64, error) {
return e.gweiValue, nil
}

// DepositReceipts --
func (e executionPayloadHeaderDeneb) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return nil, consensus_types.ErrUnsupportedField
}

// WithdrawalRequests --
func (e executionPayloadHeaderDeneb) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return nil, consensus_types.ErrUnsupportedField
}

// IsBlinded returns true if the underlying data is blinded.
func (e executionPayloadHeaderDeneb) IsBlinded() bool {
return true
Expand All @@ -1192,6 +1140,8 @@ type executionPayloadDeneb struct {
gweiValue uint64
}

var _ interfaces.ExecutionData = &executionPayloadDeneb{}

// WrappedExecutionPayloadDeneb is a constructor which wraps a protobuf execution payload into an interface.
func WrappedExecutionPayloadDeneb(p *enginev1.ExecutionPayloadDeneb, value math.Wei) (interfaces.ExecutionData, error) {
w := executionPayloadDeneb{p: p, weiValue: value, gweiValue: uint64(math.WeiToGwei(value))}
Expand Down Expand Up @@ -1344,16 +1294,6 @@ func (e executionPayloadDeneb) ValueInGwei() (uint64, error) {
return e.gweiValue, nil
}

// DepositReceipts --
func (e executionPayloadDeneb) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return nil, consensus_types.ErrUnsupportedField
}

// WithdrawalRequests --
func (e executionPayloadDeneb) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return nil, consensus_types.ErrUnsupportedField
}

// IsBlinded returns true if the underlying data is blinded.
func (e executionPayloadDeneb) IsBlinded() bool {
return false
Expand All @@ -1368,6 +1308,9 @@ type executionPayloadHeaderElectra struct {
gweiValue uint64
}

var _ interfaces.ExecutionData = &executionPayloadElectra{}
var _ interfaces.ExecutionDataElectra = &executionPayloadElectra{}

// WrappedExecutionPayloadHeaderElectra is a constructor which wraps a protobuf execution header into an interface.
func WrappedExecutionPayloadHeaderElectra(p *enginev1.ExecutionPayloadHeaderElectra, value math.Wei) (interfaces.ExecutionData, error) {
w := executionPayloadHeaderElectra{p: p, weiValue: value, gweiValue: uint64(math.WeiToGwei(value))}
Expand Down Expand Up @@ -1512,26 +1455,6 @@ func (e executionPayloadHeaderElectra) ExcessBlobGas() (uint64, error) {
return e.p.ExcessBlobGas, nil
}

// PbElectra --
func (e executionPayloadHeaderElectra) PbElectra() (*enginev1.ExecutionPayloadElectra, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PbDeneb --
func (executionPayloadHeaderElectra) PbDeneb() (*enginev1.ExecutionPayloadDeneb, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PbBellatrix --
func (executionPayloadHeaderElectra) PbBellatrix() (*enginev1.ExecutionPayload, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PbCapella --
func (executionPayloadHeaderElectra) PbCapella() (*enginev1.ExecutionPayloadCapella, error) {
return nil, consensus_types.ErrUnsupportedField
}

// ValueInWei --
func (e executionPayloadHeaderElectra) ValueInWei() (math.Wei, error) {
return e.weiValue, nil
Expand Down Expand Up @@ -1575,6 +1498,9 @@ func WrappedExecutionPayloadElectra(p *enginev1.ExecutionPayloadElectra, value m
return w, nil
}

var _ interfaces.ExecutionData = &executionPayloadElectra{}
var _ interfaces.ExecutionDataElectra = &executionPayloadElectra{}

// IsNil checks if the underlying data is nil.
func (e executionPayloadElectra) IsNil() bool {
return e.p == nil
Expand Down Expand Up @@ -1708,26 +1634,6 @@ func (e executionPayloadElectra) ExcessBlobGas() (uint64, error) {
return e.p.ExcessBlobGas, nil
}

// PbBellatrix --
func (e executionPayloadElectra) PbBellatrix() (*enginev1.ExecutionPayload, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PbCapella --
func (e executionPayloadElectra) PbCapella() (*enginev1.ExecutionPayloadCapella, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PbDeneb --
func (e executionPayloadElectra) PbDeneb() (*enginev1.ExecutionPayloadDeneb, error) {
return nil, consensus_types.ErrUnsupportedField
}

// PbElectra --
func (e executionPayloadElectra) PbElectra() (*enginev1.ExecutionPayloadElectra, error) {
return e.p, nil
}

// ValueInWei --
func (e executionPayloadElectra) ValueInWei() (math.Wei, error) {
return e.weiValue, nil
Expand All @@ -1739,13 +1645,13 @@ func (e executionPayloadElectra) ValueInGwei() (uint64, error) {
}

// DepositReceipts --
func (e executionPayloadElectra) DepositReceipts() ([]*enginev1.DepositReceipt, error) {
return e.p.DepositReceipts, nil
func (e executionPayloadElectra) DepositReceipts() []*enginev1.DepositReceipt {
return e.p.DepositReceipts
}

// WithdrawalRequests --
func (e executionPayloadElectra) WithdrawalRequests() ([]*enginev1.ExecutionLayerWithdrawalRequest, error) {
return e.p.WithdrawalRequests, nil
func (e executionPayloadElectra) WithdrawalRequests() []*enginev1.ExecutionLayerWithdrawalRequest {
return e.p.WithdrawalRequests
}

// IsBlinded returns true if the underlying data is blinded.
Expand Down
7 changes: 4 additions & 3 deletions consensus-types/blocks/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,10 +460,11 @@ func BuildSignedBeaconBlockFromExecutionPayload(blk interfaces.ReadOnlySignedBea
if err != nil {
return nil, err
}
consolidations, err := b.Body().Consolidations()
if err != nil {
return nil, err
electraBody, ok := b.Body().(interfaces.ROBlockBodyElectra)
if !ok {
return nil, errors.Wrapf(interfaces.ErrInvalidCast, "%T does not support electra getters", b.Body())
}
consolidations := electraBody.Consolidations()
var atts []*eth.AttestationElectra
if b.Body().Attestations() != nil {
atts = make([]*eth.AttestationElectra, len(b.Body().Attestations()))
Expand Down
24 changes: 24 additions & 0 deletions consensus-types/blocks/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
Expand Down Expand Up @@ -537,3 +538,26 @@ func TestBuildSignedBeaconBlockFromExecutionPayload(t *testing.T) {
require.DeepEqual(t, uint64(321), payload.BlobGasUsed)
})
}

func TestElectraBlockBodyCast(t *testing.T) {
t.Run("deneb cast fails", func(t *testing.T) {
pb := &eth.BeaconBlockBodyDeneb{}
i, err := NewBeaconBlockBody(pb)
require.NoError(t, err)
b, ok := i.(*BeaconBlockBody)
require.Equal(t, true, ok)
assert.Equal(t, version.Deneb, b.version)
_, err = interfaces.AsROBlockBodyElectra(b)
require.ErrorIs(t, err, interfaces.ErrInvalidCast)
})
t.Run("electra cast succeeds", func(t *testing.T) {
pb := &eth.BeaconBlockBodyElectra{}
i, err := NewBeaconBlockBody(pb)
require.NoError(t, err)
b, ok := i.(*BeaconBlockBody)
require.Equal(t, true, ok)
assert.Equal(t, version.Electra, b.version)
_, err = interfaces.AsROBlockBodyElectra(b)
require.NoError(t, err)
})
}
14 changes: 7 additions & 7 deletions consensus-types/blocks/getters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,11 @@ func (b *SignedBeaconBlock) ToBlinded() (interfaces.ReadOnlySignedBeaconBlock, e
Signature: b.signature[:],
})
case *enginev1.ExecutionPayloadElectra:
header, err := PayloadToHeaderElectra(payload)
pe, ok := payload.(interfaces.ExecutionDataElectra)
if !ok {
return nil, interfaces.ErrIncompatibleFork
}
header, err := PayloadToHeaderElectra(pe)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -281,7 +285,6 @@ func (b *SignedBeaconBlock) ToBlinded() (interfaces.ReadOnlySignedBeaconBlock, e
},
Signature: b.signature[:],
})

default:
return nil, fmt.Errorf("%T is not an execution payload header", p)
}
Expand Down Expand Up @@ -1170,11 +1173,8 @@ func (b *BeaconBlockBody) BlobKzgCommitments() ([][]byte, error) {
}
}

func (b *BeaconBlockBody) Consolidations() ([]*eth.SignedConsolidation, error) {
if b.version < version.Electra {
return nil, consensus_types.ErrNotSupported("Consolidations", b.version)
}
return b.signedConsolidations, nil
func (b *BeaconBlockBody) Consolidations() []*eth.SignedConsolidation {
return b.signedConsolidations
}

// Version returns the version of the beacon block body
Expand Down
Loading
Loading