conversion: replace MarshalSSZTo with MarshalSSZAppend and `Marsh…

…alSSZInto` (#171)

This change  removes `MarshalSSZTo`, to force users to choose either `MarshalSSZAppend` or `MarshalSSZInto`. The former is the future `MarshalSSZTo` which does a classic "append to the destination, and return the potentially newly allocated buffer". The latter is already deprecated, and should not be used.

conversion.go

@@ -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)
 	}
@@ -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
 }
 
@@ -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
 }
 

conversion_test.go

@@ -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 {
@@ -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) {