MarshalSSZTo and other incompatibilities with fastssz

[jshufro]

Fast ssz now expects HashTreeRootWith(hh ssz.HashWalker) (err error) in its HashTreeRoot interface. I believe the implementation is as simple as:

func (u *Uint256) HashTreeRootWith(hh ssz.HashWalker) (err error) {
        bytes := make([]byte, 32)
        bytes, err = u.MarshalSSZTo(bytes)
        if err != nil {
                return
        }

        hh.AppendBytes32(bytes)
        return
}

but I have been known to make mistakes.

Further, fastssz's MarshalSSZTo(buf []byte) ([]byte, error) interface function expects the implementation to append to the end of buf, not to overwrite it. Unfortunately, this library overwrites instead, and a wrapper is needed (one is needed anyway for sszgen to work):

type Uint256 struct {
        repr *uint256.Int `ssz:"-"`
}
[...]
func (u *Uint256) MarshalSSZTo(buf []byte) ([]byte, error) {
        bytes, err := u.repr.MarshalSSZ()
        if err != nil {
                return nil, err
        }
        return append(buf, bytes...), nil
}

[holiman]

Thanks for the heads-up!

[holiman]

Fast ssz now expects HashTreeRootWith(hh ssz.HashWalker) (err error) in its HashTreeRoot interface. I believe the implementation is as simple as:

As for this, nothing we can do here. As the comment on HashTreeRoot says, https://github.com/holiman/uint256/blob/master/conversion.go#L585 ,

// HashTreeRoot implements the fastssz.HashRoot interface's non-dependent part.

That is, out of the methods in the HashRoot interface

type HashRoot interface {
	GetTree() (*Node, error)
	HashTreeRoot() ([32]byte, error)
	HashTreeRootWith(hh HashWalker) error
}

HashTreeRot() is the only one that can be implemented without introducing a dependency from uint256 to fastssz.

[jshufro]

You know, taking a step back, the whole situation is disappointing, as the interface should expect a function that marshals ssz into a preallocated buffer. Maybe it does make([]byte, 0, N) to avoid reallocation, but that leaves you the unenviable implementation task of 1-copy serialization: first into a local buffer, then into the preallocated one.

If fastssz wants to live up to its name, maybe we can change the upstream interface to require a new MarshalSSZInto in place of MarshalSSZTo.

That way fastssz can preallocate and no extra copies need be made.

At any rate, I'm on a quick two week vacation, so i can't tell you immediately why an alias or struct with anonymous field didn't work with sszgen, but i did attempt it previously. It was before I moved the wrapper into its own package though, so maybe it works that way (aside from this bug).

[holiman]

, as the interface should expect a function that marshals ssz into a preallocated buffer.

How so? I think it's fair to expect the marshalling to append to the buffer, that's how a lot of encoders work. With ssz, we also have the size, so the caller can easily ensure that there's no realloc by just providing a buffer with correct capacity.

This is (mostly) fixed in #171 . At some point in the future, I'll add back the MarshalSSZTo method