search

ferranbt / fastssz

Fast Ethereum2.0 SSZ encoder/decoder
MIT License
74 stars 44 forks source link

Support big.Int #107

Closed henridf closed 1 year ago

henridf commented 2 years ago

It would be handy to have built-in support for big.Int (along the lines of the time.Time support added in #100).

(This is motivated by ssz-encoding geth block headers).

Would you be open to a PR doing this? If so i'll put one up.

mcdee commented 2 years ago

What would the underlying ssz type be, and how would it be represented? Would you use a uintN with an ssz-size tag to define it? And what would happen in the case of overflow?

henridf commented 2 years ago

I would probably use (*big.Int).FillBytes, like in https://github.com/henridf/eip44s-proto/blob/ca23838c532de78a279b09772286a8a443a58d08/spec/spec.go#L80.

Overflow would have to be an encoding error.

mcdee commented 2 years ago

To expand upon my first question: SSZ defines a clear set of types (basic and composite) at https://github.com/ethereum/consensus-specs/blob/dev/ssz/simple-serialize.md#basic-types so what type is big.Int representing?

big.Int should represent a type that is known in the SSZ spec, and then this library can handle the encoding and decoding of said type. Without a proper type representation in the SSZ spec it won't result in an interoperable binary format.

Is this perhaps a uint256 type?

henridf commented 2 years ago

Interesting, good point about interop. I think that both a big-endian byte slice (such as output by (*big.Int).FillBytes or a uint256 are workable here - I don't have a strong preference either way.

On Fri, 9 Sept 2022 at 20:26, Jim McDonald @.***> wrote:

To expand upon my first question: SSZ defines a clear set of types (basic and composite) at https://github.com/ethereum/consensus-specs/blob/dev/ssz/simple-serialize.md#basic-types so what type is big.Int representing?

big.Int should represent a type that is known in the SSZ spec, and then this library can handle the encoding and decoding of said type. Without a proper type representation in the SSZ spec it won't result in an interoperable binary format.

Is this perhaps a uint256 type?

— Reply to this email directly, view it on GitHub https://github.com/ferranbt/fastssz/issues/107#issuecomment-1242326379, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHZQWKL6M7YEDTHUCX5U2DV5N6N5ANCNFSM6AAAAAAQIZKLWY . You are receiving this because you authored the thread.Message ID: @.***>

henridf commented 2 years ago

Fwiw, RLP uses an approach like FillBytes: https://github.com/ethereum/go-ethereum/blob/389021a5afd01147c851870c693ded8760e6a08c/rlp/encbuffer.go#L149

ferranbt commented 1 year ago

I concur with @mcdee, we need to comply with the ssz spec, and as of now, the ssz spec does not define how a big.Int is encoded. That is something we would need to clarify first with the eth2 team (not sure in which platform) before moving ahead to avoid future compatibility issues.

Even more, there should be an agreement on how the Header itself (with Bodies and Receipts) gets encoded in ssz format (i.e. does the Number is the Header gets encoded as big.Int or uint64?) which is a side topic of this issue. Otherwise, I see a lot of edge cases being implemented in the generator to cover everything.

henridf commented 1 year ago

@ferranbt makes sense. Thinking a little more, I'm not sure SSZ needs or wants to define explicitly define arbitrary-size ints as a basic type, given the availability of uintNN and vectors.

lightclient commented 1 year ago

@ferranbt could we have big.Int be specified with an annotation like ssz-size or ssz-uint and error if the value is outside the range?