Add versioned block requests

[avalonche]

The publishBlindedBlockV2 and publishBlockV2 uses BlockContents for Deneb as request bodies.

This adds a versioned struct to support the new endpoints. Open to moving the structs to a more suitable name / location e.g. under a /v2 folder.

[mcdee]

Sorry for the delay on this, I've been considering how best to handle this. I'm not sure that versioning on the way in is a particularly suitable way to go with the expansion in types.

Given that we now have blocks, blinded blocks, block contents and blinded block contents, I'm wondering if we should have a simplified API that allows submission. Something like SubmitSignedBlock() that takes something generic (ideally we'd have something other than any, but not crippling if so) and then handles the work of deciding which endpoint to call based on the struct supplied.

This would result in a slightly tidier API long-term, and stops the world of ever-growing structs. If it works nicely we could also consider carrying over something similar to other areas where we're currently carting around versioned structs. Thoughts?

[avalonche]

my concern here is the json marshalling / unmarshalling that is not uniform across the endpoints (e.g. some endpoints have additional metadata such as version while some endpoints only contain the message). This makes it hard to unify the json marshalling without having specific logic for each endpoint and custom structs.
I think this needs to be resolved at the spec level to standardise a format containing {"metadata":..."data":...} for endpoints going forward. The versioned structs will need to have specific marshalling logic for each endpoint for the time being.

[mcdee]

All endpoints in the beacon API have a data entity for the returned object(s), with optional and response-dependent metadata. At current the codebase does not pay attention to any of the metadata items. It would be nice to have all metadata returned as well, but that would be a breaking change.

The versioning would need to continue, but only at the stage where the JSON is being unmarshalled. My comment was more focused on providing an easier interface for users and moving some of the difficulty into the codebase. Something like:

proposal := &deneb.SignedBlockContents{}
err := client.SubmitSignedProposal(ctx, proposal)

rather than the current:

proposal := &spec.VersionedSignedBeaconBlock{
  Version: spec.DataVersionDeneb,
  Deneb: &deneb.SignedBlockContents{},
}
err := client.SubmitBeaconBlock(ctx, proposal)

And then SubmitSignedProposal could run a type switch to work out what it had been passed and handle accordingly.

Basically: I'm fine with the API returning Versioned* but thinking that we shouldn't be requiring users to create them to provide to the API.

[mcdee]

I have spent some time thinking about this, and tying it in with a couple of other areas I'd like to improve. I am now considering making this even more generic and reducing it to a single Submit function for all data, for example:

// Submit submits data to the beacon node.
func (s *Service) Submit(ctx context.Context,
    data any,
    opts map[string]any,
) (
    *consensusclient.SubmitResults,
    error,
) {
    switch data.(type) {
    case *phase0.Attestation:
        return s.Submit(ctx, []*phase0.Attestation{data.(*phase0.Attestation)}, opts)
    case []*phase0.Attestation:
        return s.submitAttestations(ctx, data.([]*phase0.Attestation))
    case *phase0.SignedBeaconBlock:
        return s.submitBeaconBlock(ctx, &spec.VersionedSignedBeaconBlock{
            Version: spec.DataVersionPhase0,
            Phase0:  data.(*phase0.SignedBeaconBlock),
        })
    case *altair.SignedBeaconBlock:
        return s.submitBeaconBlock(ctx, &spec.VersionedSignedBeaconBlock{
            Version: spec.DataVersionAltair,
            Altair:  data.(*altair.SignedBeaconBlock),
        })
    case *bellatrix.SignedBeaconBlock:
        return s.submitBeaconBlock(ctx, &spec.VersionedSignedBeaconBlock{
            Version: spec.DataVersionBellatrix,
            Bellatrix:  data.(*bellatrix.SignedBeaconBlock),
        })
    case *capella.SignedBeaconBlock:
        return s.submitBeaconBlock(ctx, &spec.VersionedSignedBeaconBlock{
            Version: spec.DataVersionCapella,
            Capella:  data.(*capella.SignedBeaconBlock),
        })
    case *deneb.SignedBeaconBlock:
        return , s.submitBeaconBlock(ctx, &spec.VersionedSignedBeaconBlock{
            Version: spec.DataVersionDeneb,
            Deneb:  data.(*deneb.SignedBeaconBlock),
        })
    ...
}

(the implementation would be different, but this shows the basic idea).

The benefits of this would be:

• a single API call that handles the implementation details without the user needing to wrap objects (e.g. having to provide a VersionedSignedBeaconBlock rather than just the block they want to submit)
• better future-proofing (e.g. the change from BeaconBlock to BlockContents is jarring in the API)
• ability to provide additional options with the API calls as per the beacon API spec (e.g. providing the broadcast_validation option for block submission)
• ability to receive metadata from the API calls as per the beacon API spec as well as the returned object(s)
• retain backwards-compatibility where possible

Not sure if there would be an equivalent Obtain() function, but there may well be.

Interested in your thoughts on the above design.

[avalonche]

What would the consensusclient.SubmitResults type look like? Does submitBeaconBlock also accept something more generic or just spec.VersionedSignedBeaconBlock?
From the standpoint of the submit block api this makes sense. However this is slightly different from my use case which is deserialising the struct from a json payload.

[mcdee]

I'm going to merge this as-is, with the caveat that many of the function signatures are going to change in the next major release of this module so structs and signatures may be moved around at that point.