Add versioned attestations submission

[Bez625]

Hi Jim,

I've raised this to complement attestantio/vouch#280 and to clarify a couple of points. I've put them in the TODOs, but to summarise:

• I would appreciate your thoughts on whether (and how) we submit SingleAttestation container formats. Since it's only json going to the consensus client I wouldn't have thought the difference in payload would make much difference with regard to network usage or speed (although it's attestations so could add up over time).
• Just a sense check on the dynssz-size for CommitteeBits.

[mcdee]

I've gone back and forth on if we should have a new function here, or if we should upgrade the signature of the existing one. I'm now coming down on the side of the latter, so thinking that we can change the signature to SubmitAttestations(ctx,opts) rather than have multiple functions. Yes it's a breaking change, but we're still at version 0.x of the library and I suspect that there are very few codebases that use the submission functions.

My understanding of the v2 endpoint is that we should send Attestation objects up to the electra hard fork, and from thereon in send SingleAttestation objects. Given that the library is broadly unaware of the hard forks, we should do this based purely on the version of the versioned attestation objects. Once we're up and running with this in Vouch we can then take a closer look at the behavior of the various beacon nodes to see how well this works.

For the dynamic SSZ tags you can check https://github.com/pk910/dynamic-ssz which is the codebase that uses them.

[Bez625]

Thanks for the comments @mcdee - I have refactored as suggested, please take a look once you have time.

[mcdee]

Looking good; a few comments.

[mcdee]

We don't need to use a pointer here, the enum has a DataVersionUnknown which we can use as the undefined value rather than nil in the later check, and that then removes the dereferencing.

[mcdee]

I think that this needs a dynssz tag of MAX_VALIDATORS_PER_COMMITTEE * MAX_COMMITTEES_PER_SLOT.

Also, looking at fastssz I think that ssz-max should be 131072 i.e. the maximum number of bits. It looks like the library takes care of calculating byte-level sizes from this tag.

[Bez625]

I wanted to convince myself so I added the test here to prove that this works with the current values and that it only references the array size.

The MAX_VALIDATORS_PER_COMMITTEE * MAX_COMMITTEES_PER_SLOT is interesting. After the above experiment I actually arrived at the value ((MAX_VALIDATORS_PER_COMMITTEE * MAX_COMMITTEES_PER_SLOT)/8 + 1), but when I tried to use it I encountered an issue with code generation.

When I tried to generate the ssz code with this definition:

type Attestation struct {
	// bitfield.Bitlist has size of n bits + 1 length bit, e.g. an 8 bit list will require a 2 byte array.
	AggregationBits bitfield.Bitlist `dynssz-size:"((MAX_VALIDATORS_PER_COMMITTEE * MAX_COMMITTEES_PER_SLOT)/8 + 1)" ssz-size:"16385"`
	Data            *phase0.AttestationData
	Signature       phase0.BLSSignature `ssz-size:"96"`
	// bitfield.Bitvector64 is an 8 byte array so dynamic sizing doesn't make sense.
	CommitteeBits bitfield.Bitvector64 `ssz-size:"8"`
}

I get this error:

[ERR]: failed to encode Attestation: bitlist AggregationBits does not have ssz-max tag
generate.go:19: running "sszgen": exit status 1

Which I presume is because bitfield.Bitlist has an unbound size. I looked in the docs and I couldnt see any mention of ssz-max in the dynamic-ssz library, so I don't know what will happen in this situation where the realised dynamic size is higher than the max, but I presume the fastssz max will be enforced.

At this stage I didn't see the point in adding the dynssz-size tag if we have to explicitly fix the max size of it anyway.

[mcdee]

ssz-size and ssz-max are for the static SSZ generation, dynssz-size is for the dynamic SSZ calculation, so we should have both (which one we use depends on the value passed to WithCustomSpecSupport()).

Regarding the size, what you calculated is the number of bytes in the bitlist representation (as it has a trailing 1 bit to mark the end of the list) but I believe that we don't need to carry out this calculation as the SSZ and bitfield libraries do this for us already (hence the phase0 value being 131072, which is the maximum size in bits).

[mcdee]

And one other note: although bitlists themselves don't have a max, every list in the Ethereum spec does have a max because it is required for SSZ encoding.

[Bez625]

Understood on dynamic SSZ calculation, I've added the dynssz-size to the AggregateBits.

Regarding the size, what you calculated is the number of bytes in the bitlist representation (as it has a trailing 1 bit to mark the end of the list) but I believe that we don't need to carry out this calculation as the SSZ and bitfield libraries do this for us already

If I set the ssz-max to 16384 I get this marshalling error with a bitlist of size 131072:

Attestation.AggregationBits (bytes array does not have the correct length): expected 16384 and 16385 found

If I set the ssz-max to 131,072 it will allow serialisation of a bitlist of size 1,048,575 bits.

[Bez625]

You can see why in the generated code:

	if size := len(a.AggregationBits); size > 16385 {
		err = ssz.ErrBytesLengthFn("Attestation.AggregationBits", size, 16385)
		return
	}

It's using len() instead of AggregationBits.Len(). Although len() gives you number of bytes and .Len() gives you number of bits.

[mcdee]

I think we should keep this as 131072. Although from the sounds of it, it doesn't do what it should, we should raise the issue with the fastssz repository and have the correct values in place here for when the bug is fixed.