Repository navigation
MarshalSSZTo and other incompatibilities with fastssz #170
Description
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
}
Activity
holiman commentedon May 21, 2024
Thanks for the heads-up!
holiman commentedon May 21, 2024
As for this, nothing we can do here. As the comment on
HashTreeRootsays, https://github.com/holiman/uint256/blob/master/conversion.go#L585 ,That is, out of the methods in the
HashRootinterfaceHashTreeRot()is the only one that can be implemented without introducing a dependency fromuint256tofastssz.jshufro commentedon Jun 1, 2024
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
MarshalSSZIntoin place ofMarshalSSZTo.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 commentedon Jun 12, 2024
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
MarshalSSZTomethod