Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conversion: replace MarshalSSZTo with MarshalSSZAppend and MarshalSSZInto #171

Merged
merged 3 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,9 +540,41 @@ func bigEndianUint56(b []byte) uint64 {
uint64(b[2])<<32 | uint64(b[1])<<40 | uint64(b[0])<<48
}

// MarshalSSZTo implements the fastssz.Marshaler interface and serializes the
// integer into an already pre-allocated buffer.
func (z *Int) MarshalSSZTo(dst []byte) ([]byte, error) {
// MarshalSSZAppend _almost_ implements the fastssz.Marshaler (see below) interface.
// Originally, this method was named `MarshalSSZTo`, and ostensibly implemented the interface.
// However, it was noted (https://github.com/holiman/uint256/issues/170) that the
// actual implementation did not match the intended semantics of the interface: it
// inserted the data instead of appending.
//
// Therefore, the `MarshalSSZTo` has been removed: to force users into making a choice:
// - Use `MarshalSSZAppend`: this is the method that appends to the destination buffer,
// and returns a potentially newly allocated buffer. This method will become `MarshalSSZTo`
// in some future release.
// - Or use `MarshalSSZInto`: this is the original method which places the data into
// the destination buffer, without ever reallocating.
//
// fastssz.Marshaler interface:
//
// https://github.com/ferranbt/fastssz/blob/main/interface.go#L4
// type Marshaler interface {
// MarshalSSZTo(dst []byte) ([]byte, error)
// MarshalSSZ() ([]byte, error)
// SizeSSZ() int
// }
func (z *Int) MarshalSSZAppend(dst []byte) ([]byte, error) {
dst = binary.LittleEndian.AppendUint64(dst, z[0])
dst = binary.LittleEndian.AppendUint64(dst, z[1])
dst = binary.LittleEndian.AppendUint64(dst, z[2])
dst = binary.LittleEndian.AppendUint64(dst, z[3])
return dst, nil
}

// MarshalSSZInto is the first attempt to implement the fastssz.Marshaler interface,
// but which does not obey the intended semantics. See MarshalSSZAppend and
// - https://github.com/holiman/uint256/pull/171
// - https://github.com/holiman/uint256/issues/170
// @deprecated
func (z *Int) MarshalSSZInto(dst []byte) ([]byte, error) {
if len(dst) < 32 {
return nil, fmt.Errorf("%w: have %d, want %d bytes", ErrBadBufferLength, len(dst), 32)
}
Expand All @@ -557,8 +589,7 @@ func (z *Int) MarshalSSZTo(dst []byte) ([]byte, error) {
// MarshalSSZ implements the fastssz.Marshaler interface and returns the integer
// marshalled into a newly allocated byte slice.
func (z *Int) MarshalSSZ() ([]byte, error) {
blob := make([]byte, 32)
_, _ = z.MarshalSSZTo(blob) // ignore error, cannot fail, surely have 32 byte space in blob
blob, _ := z.MarshalSSZAppend(make([]byte, 0, 32)) // ignore error, cannot fail, surely have 32 byte space in blob
return blob, nil
}

Expand All @@ -584,8 +615,9 @@ func (z *Int) UnmarshalSSZ(buf []byte) error {

// HashTreeRoot implements the fastssz.HashRoot interface's non-dependent part.
func (z *Int) HashTreeRoot() ([32]byte, error) {
b, _ := z.MarshalSSZAppend(make([]byte, 0, 32)) // ignore error, cannot fail
var hash [32]byte
_, _ = z.MarshalSSZTo(hash[:]) // ignore error, cannot fail
copy(hash[:], b)
return hash, nil
}

Expand Down
11 changes: 7 additions & 4 deletions conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ func TestSSZEncodeDecodeHash(t *testing.T) {
t.Fatal(err)
}
if exp := hex2Bytes(tt.exp); !bytes.Equal(b, exp) {
t.Errorf("testcase %d, encoding got:\n%x\nexp:%x\n", i, b, exp)
t.Errorf("testcase %d, encoding\nhave: %x\nwant: %x\n", i, b, exp)
}
z2 := new(Int)
if err := z2.UnmarshalSSZ(b); err != nil {
Expand All @@ -733,26 +733,29 @@ func TestSSZEncodeDecodeHash(t *testing.T) {
t.Fatal(err)
}
if exp := hex2Bytes(tt.exp); !bytes.Equal(r[:], exp) {
t.Errorf("testcase %d, hashing got:\n%x\nexp:%x\n", i, r, exp)
t.Errorf("testcase %d, hashing\nhave: %x\nwant: %x\n", i, r, exp)
}
}
}

func TestSSZEncodeDecodeErrors(t *testing.T) {
small := make([]byte, 31)
if _, err := new(Int).MarshalSSZTo(small); !errors.Is(err, ErrBadBufferLength) {
if _, err := new(Int).MarshalSSZInto(small); !errors.Is(err, ErrBadBufferLength) {
t.Fatalf("overflow marshal error mismatch: have %v, want %v", err, ErrBadBufferLength)
}
if err := new(Int).UnmarshalSSZ(small); !errors.Is(err, ErrBadEncodedLength) {
t.Fatalf("underflow unmarshal error mismatch: have %v, want %v", err, ErrBadEncodedLength)
}
large := make([]byte, 33)
if _, err := new(Int).MarshalSSZTo(large); err != nil {
if _, err := new(Int).MarshalSSZAppend(large); err != nil {
t.Fatalf("underflow marshal error mismatch: have %v, want %v", err, nil)
}
if err := new(Int).UnmarshalSSZ(large); !errors.Is(err, ErrBadEncodedLength) {
t.Fatalf("overflow unmarshal error mismatch: have %v, want %v", err, ErrBadEncodedLength)
}
if _, err := new(Int).MarshalSSZInto(large); err != nil {
t.Fatalf("unexpected error: %v", err)
}
}

func TestRLPEncode(t *testing.T) {
Expand Down