strkey: reduce allocs in Encode (#4107)

Replace bytes.Buffer with working with a byte array directly.

Also, contains the crc16 improvements by @2opremio that were in #4105.

Reduce the allocations in strkey.Encode from 6 to 1, and reduce the heap use from 244 B/op to 64 B/op.

In the past I have seen impact to performance of Starlight caused by the strkey.Encode functions. @2opremio also highlighted in #4105 there is an impact there.

The use of a bytes.Buffer in this code is rather unnecessary given we know the size of everything, and given that the size of strkeys are small encouraging their data onto the stack is fine.

It is worth noting that there is a functional impact by this change. Attempts to strkey.Encode a data value greater than the max payload size as defined by SEP-23 will now error. We've been wanting to add length checks (#1769) for sometime, so I think this is fine, and we can continue to improve those length checks in the future. 

Before:
BenchmarkDecode_accountID-8      2576450               414.2 ns/op           130 B/op          3 allocs/op
BenchmarkEncode_accountID-8      3657649               319.6 ns/op           244 B/op          6 allocs/op

After:
BenchmarkDecode_accountID-8      3563737               334.9 ns/op           128 B/op          2 allocs/op
BenchmarkEncode_accountID-8      7365306               165.4 ns/op            64 B/op          1 allocs/op

Co-authored-by: Alfonso Acosta <2362916+2opremio@users.noreply.github.com>

strkey/benchmark_test.go

@@ -0,0 +1,23 @@
+package strkey_test
+
+import (
+	"testing"
+
+	"github.com/stellar/go/strkey"
+	"github.com/stretchr/testify/require"
+)
+
+func BenchmarkDecode_accountID(b *testing.B) {
+	accountID, err := strkey.Encode(strkey.VersionByteAccountID, make([]byte, 32))
+	require.NoError(b, err)
+	for i := 0; i < b.N; i++ {
+		_, _ = strkey.Decode(strkey.VersionByteAccountID, accountID)
+	}
+}
+
+func BenchmarkEncode_accountID(b *testing.B) {
+	accountID := make([]byte, 32)
+	for i := 0; i < b.N; i++ {
+		_, _ = strkey.Encode(strkey.VersionByteAccountID, accountID)
+	}
+}

strkey/encode_test.go

@@ -81,4 +81,15 @@ func TestEncode(t *testing.T) {
 	// test bad version byte
 	_, err := Encode(VersionByte(2), cases[0].Payload)
 	assert.Error(t, err)
+
+	// test too long payload
+	_, err = Encode(VersionByteAccountID, []byte{
+		0x69, 0xa8, 0xc4, 0xcb, 0xb9, 0xf6, 0x4e, 0x8a,
+		0x07, 0x98, 0xf6, 0xe1, 0xac, 0x65, 0xd0, 0x6c,
+		0x31, 0x62, 0x92, 0x90, 0x56, 0xbc, 0xf4, 0xcd,
+		0xb7, 0xd3, 0x73, 0x8d, 0x18, 0x55, 0xf3, 0x63,
+		0x69, 0xa8, 0xc4, 0xcb, 0xb9, 0xf6, 0x4e, 0x8a,
+		0x01,
+	})
+	assert.EqualError(t, err, "data exceeds maximum payload size for strkey")
 }

strkey/internal/crc16/main.go

@@ -47,8 +47,6 @@
 package crc16
 
 import (
-	"bytes"
-	"encoding/binary"
 	"errors"
 )
 
@@ -92,26 +90,21 @@ var crc16tab = [256]uint16{
 }
 
 // Checksum returns the 2-byte checksum for the provided data
-func Checksum(data []byte) []byte {
+func Checksum(data []byte) uint16 {
 	var crc uint16
-	var out [2]byte
 	for _, b := range data {
 		crc = ((crc << 8) & 0xffff) ^ crc16tab[((crc>>8)^uint16(b))&0x00FF]
 	}
-
-	binary.LittleEndian.PutUint16(out[:], crc)
-
-	return out[:]
+	return crc
 }
 
 // Validate returns an error if the provided checksum does not match
 // the calculated checksum of the provided data
-func Validate(data []byte, expected []byte) error {
-
+func Validate(data []byte, expected uint16) error {
 	actual := Checksum(data)
 
 	// validate the provided checksum against the calculated
-	if !bytes.Equal(actual, expected) {
+	if actual != expected {
 		return ErrInvalidChecksum
 	}
 

strkey/internal/crc16/main_test.go

@@ -8,13 +8,13 @@ import (
 
 func TestChecksum(t *testing.T) {
 	result := Checksum([]byte{0x12, 0x34, 0x56, 0x78, 0x90})
-	assert.Equal(t, []byte{0xe6, 0x48}, result)
+	assert.Equal(t, uint16(0x48e6), result)
 }
 
 func TestValidate(t *testing.T) {
-	err := Validate([]byte{0x12, 0x34, 0x56, 0x78, 0x90}, []byte{0xe6, 0x48})
+	err := Validate([]byte{0x12, 0x34, 0x56, 0x78, 0x90}, 0x48e6)
 	assert.NoError(t, err)
 
-	err = Validate([]byte{0x12, 0x34, 0x56, 0x78, 0x90}, []byte{0xe7, 0x48})
+	err = Validate([]byte{0x12, 0x34, 0x56, 0x78, 0x90}, 0x48e7)
 	assert.ErrorIs(t, err, ErrInvalidChecksum)
 }

strkey/main.go

@@ -1,9 +1,9 @@
 package strkey
 
 import (
-	"bytes"
 	"encoding/base32"
 	"encoding/binary"
+	"fmt"
 
 	"github.com/stellar/go/strkey/internal/crc16"
 	"github.com/stellar/go/support/errors"
@@ -36,6 +36,18 @@ const (
 	VersionByteHashX = 23 << 3 // Base32-encodes to 'X...'
 )
 
+// maxPayloadSize is the maximum length of the payload for all versions.
+const maxPayloadSize = 40
+
+// maxRawSize is the maximum length of a strkey in its raw form not encoded.
+const maxRawSize = 1 + maxPayloadSize + 2
+
+// maxEncodedSize is the maximum length of a strkey when base32 encoded.
+const maxEncodedSize = (maxRawSize*8 + 4) / 5 // (8n+4)/5 is the EncodedLen for no padding
+
+// encoding to use when encoding and decoding a strkey to and from strings.
+var encoding = base32.StdEncoding.WithPadding(base32.NoPadding)
+
 // DecodeAny decodes the provided StrKey into a raw value, checking the checksum
 // and if the version byte is one of allowed values.
 func DecodeAny(src string) (VersionByte, []byte, error) {
@@ -56,7 +68,7 @@ func DecodeAny(src string) (VersionByte, []byte, error) {
 	}
 
 	// ensure checksum is valid
-	if err := crc16.Validate(vp, checksum); err != nil {
+	if err := crc16.Validate(vp, binary.LittleEndian.Uint16(checksum)); err != nil {
 		return 0, nil, err
 	}
 
@@ -94,7 +106,7 @@ func Decode(expected VersionByte, src string) ([]byte, error) {
 	}
 
 	// ensure checksum is valid
-	if err := crc16.Validate(vp, checksum); err != nil {
+	if err := crc16.Validate(vp, binary.LittleEndian.Uint16(checksum)); err != nil {
 		return nil, err
 	}
 
@@ -118,26 +130,32 @@ func Encode(version VersionByte, src []byte) (string, error) {
 		return "", err
 	}
 
-	var raw bytes.Buffer
+	payloadSize := len(src)
 
-	// write version byte
-	if err := binary.Write(&raw, binary.LittleEndian, version); err != nil {
-		return "", err
-	}
-
-	// write payload
-	if _, err := raw.Write(src); err != nil {
-		return "", err
-	}
-
-	// calculate and write checksum
-	checksum := crc16.Checksum(raw.Bytes())
-	if _, err := raw.Write(checksum); err != nil {
-		return "", err
+	// check src does not exceed maximum payload size
+	if payloadSize > maxPayloadSize {
+		return "", fmt.Errorf("data exceeds maximum payload size for strkey")
 	}
 
-	result := base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(raw.Bytes())
-	return result, nil
+	// pack
+	//  1 byte version
+	//  src bytes
+	//  2 byte crc16
+	rawArr := [maxRawSize]byte{}
+	rawSize := 1 + payloadSize + 2
+	raw := rawArr[:rawSize]
+	raw[0] = byte(version)
+	copy(raw[1:], src)
+	crc := crc16.Checksum(raw[:1+payloadSize])
+	binary.LittleEndian.PutUint16(raw[1+payloadSize:], crc)
+
+	// base32 encode
+	encArr := [maxEncodedSize]byte{}
+	encSize := encoding.EncodedLen(rawSize)
+	enc := encArr[:encSize]
+	encoding.Encode(enc, raw)
+
+	return string(enc), nil
 }
 
 // MustEncode is like Encode, but panics on error

strkey/main_test.go

@@ -6,6 +6,10 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestMaxEncodedSize(t *testing.T) {
+	assert.Equal(t, encoding.EncodedLen(maxRawSize), maxEncodedSize)
+}
+
 func TestVersion(t *testing.T) {
 	cases := []struct {
 		Name                string