Bigint and bitfield-as-integer

[vbuterin]

Adds bigint and bitfield support to SSZ.

Possible alternative definition for Bitfield:

o = [0] * (len(value)//8 + 1)
for i in range(len(value)):
    o[i//8] ^= value[i] << (i%8)
o[len(value)//8] ^= 1 << (len(value) % 8)
return bytes(o)

[dankrad]

Is there a good reason for why we want to support Bigints? Otherwise I would tend to favour the alternative definition over introducing Bigints just for defining Bitfields. While the definition is more elegant with Bigints, I think in reality all implementations will use this.

[vbuterin]

If we want to use SSZ as an ABI later on, then there will definitely be use cases for bigints, so there's arguably a reason to have them in there. But you're right, it's not necessary for consensus layer.

[dankrad]

I think it makes sense to add this, we've already opted for union types on similar grounds. Also, I quite like the "Bitfield as Bigint" definition because where it will come very natural is in MPCs where it is very natural to operate on Bitfields in this way.
I tried adding the Bigint and Bitfield to the other sections of the document. Clearly they should no be a basic type as they are variable length. Not entirely happy to classify them as "composite", if someone has a better name ("Helper type" to indicate that it is only a thin layer on top of "bytes"?) maybe we should split this section.
I suggest capitalising these types, in fact I would suggest capitalising all non-basic types (at the moment this is inconsistent).

[djrtwo]

Another argument for the bigint types was to be able to handle deeper SSZ objects in generalized merkle indices ( #842 (comment))

[dankrad]

Oh, and this request would basically only support variable size Bitfields, right? Some of the potential applications in the spec seem to be fixed size, should we add support for this too?

[djrtwo]

This looks good. We should modify the justified_bitfield type before merging

[dankrad]

As per our discussion yesterday, added a fixed length Bitfield. Not really sure about the notation I'm using; some alternatives:

• Abuse the list/vector notation and use ["bool"] and ["bool", N] for variable/fixed length bit fields. Then there would be no way to do get them in the "one byte per bool" version anymore (but do we even need that?) -- I guess this is very close to what @JustinDrake suggested on the call, at least in terms of notation
• Alternative names for variable/fixed length bitfields if not abusing notation:
  • VariableBitfield and FixedBitfield[N] (my current suggestion)
  • Bitlist and Bitvector[N]
  • Bitfield and Bitfield[N] (I found this a bit "dirty")

[protolambda]

Now that #1077 is merged and we have Vector[123] and BytesN[123], I don't think Bitfield[123] is very strange. The bad part however is that we don't want Bitfield to be of the same type as Bitfield[42]. So I prefer the Bitlist + Bitvector[N] naming.

My recommendation would be to not push it to the SSZ layer. And instead, just implement the data types as subclasses of bytes and BytesN[X]. And then make the item lookup function and concatenation deal with the i / 8 to map it into bytes. This way neither the spec nor the ssz implementation will be affected :)

Edit: not sure about the big-int usage, but understand consistent formatting/hashing of big-ints (no leading zero bytes) is important for consensus. So that may need to be addressed in SSZ. If we're going that route, it may be better to wrap bitlist and bitvector around that type, instead of bytes/BytesN.

[protolambda]

Just a quick nitpick: the code is full of off-by-1 errors (missing the partially used byte). E.g. log2(x)//8, should be (a.bit_length() + 7) // 8, but yes, it's less pretty. It is probably a good idea to just write the conversion function once, and re-use that.

[djrtwo]

@dankrad Can we close this in favor of your coming PR?

[dankrad]

Yes, please :)