refactor: removing redundant codes in htrutils.go (#15453)

* refactor: use auto-generated HashTreeRoot functions in htrutil.go

* refactor: use type alias for Transaction & use SliceRoot for TransactionsRoot

* changelog

* fix: TransactionsRoot receives raw 2d bytes as an argument

* fix: handle nil argument

* test: add nil test for fork and checkpoint

---------

Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>

changelog/syjn99_refactor-htrutil.md

@@ -0,0 +1,3 @@
+### Changed
+
+- Refactor `htrutil.go` by removing redundant codes.

encoding/ssz/htrutils.go

@@ -11,6 +11,12 @@ import (
 	"github.com/pkg/errors"
 )
 
+type Transaction []byte
+
+func (t Transaction) HashTreeRoot() ([32]byte, error) {
+	return ByteSliceRoot(t, fieldparams.MaxBytesPerTxLength)
+}
+
 // Uint64Root computes the HashTreeRoot Merkleization of
 // a simple uint64 value according to the Ethereum
 // Simple Serialize specification.
@@ -21,33 +27,22 @@ func Uint64Root(val uint64) [32]byte {
 	return root
 }
 
-// ForkRoot computes the HashTreeRoot Merkleization of
+// ForkRoot computes the HashTreeRoot Merkleization of Fork
-// a Fork struct value according to the Ethereum
-// Simple Serialize specification.
 func ForkRoot(fork *ethpb.Fork) ([32]byte, error) {
-	fieldRoots := make([][32]byte, 3)
+	if fork == nil {
-	if fork != nil {
+		fieldRoots := make([][32]byte, 3)
-		fieldRoots[0] = bytesutil.ToBytes32(fork.PreviousVersion)
+		return BitwiseMerkleize(fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
-		fieldRoots[1] = bytesutil.ToBytes32(fork.CurrentVersion)
-		forkEpochBuf := make([]byte, 8)
-		binary.LittleEndian.PutUint64(forkEpochBuf, uint64(fork.Epoch))
-		fieldRoots[2] = bytesutil.ToBytes32(forkEpochBuf)
 	}
-	return BitwiseMerkleize(fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
+	return fork.HashTreeRoot()
 }
 
-// CheckpointRoot computes the HashTreeRoot Merkleization of
+// CheckpointRoot computes the HashTreeRoot Merkleization of Checkpoint
-// a InitWithReset struct value according to the Ethereum
-// Simple Serialize specification.
 func CheckpointRoot(checkpoint *ethpb.Checkpoint) ([32]byte, error) {
-	fieldRoots := make([][32]byte, 2)
+	if checkpoint == nil {
-	if checkpoint != nil {
+		fieldRoots := make([][32]byte, 2)
-		epochBuf := make([]byte, 8)
+		return BitwiseMerkleize(fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
-		binary.LittleEndian.PutUint64(epochBuf, uint64(checkpoint.Epoch))
-		fieldRoots[0] = bytesutil.ToBytes32(epochBuf)
-		fieldRoots[1] = bytesutil.ToBytes32(checkpoint.Root)
 	}
-	return BitwiseMerkleize(fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
+	return checkpoint.HashTreeRoot()
 }
 
 // ByteArrayRootWithLimit computes the HashTreeRoot Merkleization of
@@ -91,54 +86,18 @@ func SlashingsRoot(slashings []uint64) ([32]byte, error) {
 }
 
 // TransactionsRoot computes the HTR for the Transactions' property of the ExecutionPayload
-// The code was largely copy/pasted from the code generated to compute the HTR of the entire
-// ExecutionPayload.
 func TransactionsRoot(txs [][]byte) ([32]byte, error) {
-	txRoots := make([][32]byte, 0)
+	transactions := make([]Transaction, len(txs))
-	for i := 0; i < len(txs); i++ {
+	for i, tx := range txs {
-		rt, err := ByteSliceRoot(txs[i], fieldparams.MaxBytesPerTxLength) // getting the transaction root here
+		transactions[i] = Transaction(tx)
-		if err != nil {
-			return [32]byte{}, err
-		}
-		txRoots = append(txRoots, rt)
 	}
-
+	return SliceRoot(transactions, fieldparams.MaxTxsPerPayloadLength)
-	bytesRoot, err := BitwiseMerkleize(txRoots, uint64(len(txRoots)), fieldparams.MaxTxsPerPayloadLength)
-	if err != nil {
-		return [32]byte{}, errors.Wrap(err, "could not compute  merkleization")
-	}
-	bytesRootBuf := new(bytes.Buffer)
-	if err := binary.Write(bytesRootBuf, binary.LittleEndian, uint64(len(txs))); err != nil {
-		return [32]byte{}, errors.Wrap(err, "could not marshal length")
-	}
-	bytesRootBufRoot := make([]byte, 32)
-	copy(bytesRootBufRoot, bytesRootBuf.Bytes())
-	return MixInLength(bytesRoot, bytesRootBufRoot), nil
 }
 
 // WithdrawalSliceRoot computes the HTR of a slice of withdrawals.
 // The limit parameter is used as input to the bitwise merkleization algorithm.
 func WithdrawalSliceRoot(withdrawals []*enginev1.Withdrawal, limit uint64) ([32]byte, error) {
-	roots := make([][32]byte, len(withdrawals))
+	return SliceRoot(withdrawals, limit)
-	for i := 0; i < len(withdrawals); i++ {
-		r, err := withdrawalRoot(withdrawals[i])
-		if err != nil {
-			return [32]byte{}, err
-		}
-		roots[i] = r
-	}
-
-	bytesRoot, err := BitwiseMerkleize(roots, uint64(len(roots)), limit)
-	if err != nil {
-		return [32]byte{}, errors.Wrap(err, "could not compute  merkleization")
-	}
-	bytesRootBuf := new(bytes.Buffer)
-	if err := binary.Write(bytesRootBuf, binary.LittleEndian, uint64(len(withdrawals))); err != nil {
-		return [32]byte{}, errors.Wrap(err, "could not marshal length")
-	}
-	bytesRootBufRoot := make([]byte, 32)
-	copy(bytesRootBufRoot, bytesRootBuf.Bytes())
-	return MixInLength(bytesRoot, bytesRootBufRoot), nil
 }
 
 // DepositRequestsSliceRoot computes the HTR of a slice of deposit requests.
@@ -176,14 +135,9 @@ func ByteSliceRoot(slice []byte, maxLength uint64) ([32]byte, error) {
 }
 
 func withdrawalRoot(w *enginev1.Withdrawal) ([32]byte, error) {
-	fieldRoots := make([][32]byte, 4)
+	if w == nil {
-	if w != nil {
+		fieldRoots := make([][32]byte, 4)
-		binary.LittleEndian.PutUint64(fieldRoots[0][:], w.Index)
+		return BitwiseMerkleize(fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
-
-		binary.LittleEndian.PutUint64(fieldRoots[1][:], uint64(w.ValidatorIndex))
-
-		fieldRoots[2] = bytesutil.ToBytes32(w.Address)
-		binary.LittleEndian.PutUint64(fieldRoots[3][:], w.Amount)
 	}
-	return BitwiseMerkleize(fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
+	return w.HashTreeRoot()
 }

encoding/ssz/htrutils_test.go

@@ -22,28 +22,62 @@ func TestUint64Root(t *testing.T) {
 }
 
 func TestForkRoot(t *testing.T) {
-	testFork := ethpb.Fork{
+	tests := []struct {
-		PreviousVersion: []byte{123},
+		name     string
-		CurrentVersion:  []byte{124},
+		fork     *ethpb.Fork
-		Epoch:           1234567890,
+		expected [32]byte
+	}{
+		{
+			name:     "nil",
+			fork:     nil,
+			expected: [32]byte{219, 86, 17, 78, 0, 253, 212, 193, 248, 92, 137, 43, 243, 90, 201, 168, 146, 137, 170, 236, 177, 235, 208, 169, 108, 222, 96, 106, 116, 139, 93, 113},
+		},
+		{
+			name: "valid fork",
+			fork: &ethpb.Fork{
+				PreviousVersion: []byte{123, 0, 0, 0},
+				CurrentVersion:  []byte{124, 0, 0, 0},
+				Epoch:           1234567890,
+			},
+			expected: [32]byte{19, 46, 77, 103, 92, 175, 247, 33, 100, 64, 17, 111, 199, 145, 69, 38, 217, 112, 6, 16, 149, 201, 225, 144, 192, 228, 197, 172, 157, 78, 114, 140},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := ssz.ForkRoot(tt.fork)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, result)
+		})
 	}
-	expected := [32]byte{19, 46, 77, 103, 92, 175, 247, 33, 100, 64, 17, 111, 199, 145, 69, 38, 217, 112, 6, 16, 149, 201, 225, 144, 192, 228, 197, 172, 157, 78, 114, 140}
-
-	result, err := ssz.ForkRoot(&testFork)
-	require.NoError(t, err)
-	assert.Equal(t, expected, result)
 }
 
 func TestCheckPointRoot(t *testing.T) {
-	testCheckpoint := ethpb.Checkpoint{
+	tests := []struct {
-		Epoch: 1234567890,
+		name       string
-		Root:  []byte{222},
+		checkpoint *ethpb.Checkpoint
+		expected   [32]byte
+	}{
+		{
+			name:       "nil",
+			checkpoint: nil,
+			expected:   [32]byte{245, 165, 253, 66, 209, 106, 32, 48, 39, 152, 239, 110, 211, 9, 151, 155, 67, 0, 61, 35, 32, 217, 240, 232, 234, 152, 49, 169, 39, 89, 251, 75},
+		},
+		{
+			name: "valid checkpoint",
+			checkpoint: &ethpb.Checkpoint{
+				Epoch: 1234567890,
+				Root:  []byte{222, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+			},
+			expected: [32]byte{228, 65, 39, 109, 183, 249, 167, 232, 125, 239, 25, 155, 207, 4, 84, 174, 176, 229, 175, 224, 62, 33, 215, 254, 170, 220, 132, 65, 246, 128, 68, 194},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := ssz.CheckpointRoot(tt.checkpoint)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, result)
+		})
 	}
-	expected := [32]byte{228, 65, 39, 109, 183, 249, 167, 232, 125, 239, 25, 155, 207, 4, 84, 174, 176, 229, 175, 224, 62, 33, 215, 254, 170, 220, 132, 65, 246, 128, 68, 194}
-
-	result, err := ssz.CheckpointRoot(&testCheckpoint)
-	require.NoError(t, err)
-	assert.Equal(t, expected, result)
 }
 
 func TestByteArrayRootWithLimit(t *testing.T) {
@@ -219,7 +253,7 @@ func TestWithdrawalRoot(t *testing.T) {
 	}{
 		{
 			name:  "nil",
-			input: &enginev1.Withdrawal{},
+			input: nil,
 			want:  [32]byte{0xdb, 0x56, 0x11, 0x4e, 0x0, 0xfd, 0xd4, 0xc1, 0xf8, 0x5c, 0x89, 0x2b, 0xf3, 0x5a, 0xc9, 0xa8, 0x92, 0x89, 0xaa, 0xec, 0xb1, 0xeb, 0xd0, 0xa9, 0x6c, 0xde, 0x60, 0x6a, 0x74, 0x8b, 0x5d, 0x71},
 		},
 		{