Merge branch 'cleaning-up-payload-handling' of github.com:prysmaticlabs/prysm into cleaning-up-payload-handling

beacon-chain/core/blocks/payload.go

@@ -242,34 +242,12 @@ func ValidatePayloadHeaderWhenMergeCompletes(st state.BeaconState, header interf
 	return nil
 }
 
-// ValidatePayloadHeader validates the payload header.
-func ValidatePayloadHeader(st state.BeaconState, header interfaces.ExecutionData) error {
-	// Validate header's random mix matches with state in current epoch
-	random, err := helpers.RandaoMix(st, time.CurrentEpoch(st))
-	if err != nil {
-		return err
-	}
-	if !bytes.Equal(header.PrevRandao(), random) {
-		return ErrInvalidPayloadPrevRandao
-	}
-
-	// Validate header's timestamp matches with state in current slot.
-	t, err := slots.ToTime(st.GenesisTime(), st.Slot())
-	if err != nil {
-		return err
-	}
-	if header.Timestamp() != uint64(t.Unix()) {
-		return ErrInvalidPayloadTimeStamp
-	}
-	return nil
-}
-
 // ProcessPayloadHeader processes the payload header.
 func ProcessPayloadHeader(st state.BeaconState, header interfaces.ExecutionData) (state.BeaconState, error) {
 	if err := ValidatePayloadHeaderWhenMergeCompletes(st, header); err != nil {
 		return nil, err
 	}
-	if err := ValidatePayloadHeader(st, header); err != nil {
+	if err := ValidatePayload(st, header); err != nil {
 		return nil, err
 	}
 	if err := st.SetLatestExecutionPayloadHeader(header); err != nil {

beacon-chain/core/blocks/payload_test.go

@@ -564,52 +564,6 @@ func Test_ProcessPayloadHeader(t *testing.T) {
 	}
 }
 
-func Test_ValidatePayloadHeader(t *testing.T) {
-	st, _ := util.DeterministicGenesisStateBellatrix(t, 1)
-	random, err := helpers.RandaoMix(st, time.CurrentEpoch(st))
-	require.NoError(t, err)
-	ts, err := slots.ToTime(st.GenesisTime(), st.Slot())
-	require.NoError(t, err)
-	tests := []struct {
-		name   string
-		header *enginev1.ExecutionPayloadHeader
-		err    error
-	}{
-		{
-			name: "process passes",
-			header: func() *enginev1.ExecutionPayloadHeader {
-				h := emptyPayloadHeader()
-				h.PrevRandao = random
-				h.Timestamp = uint64(ts.Unix())
-				return h
-			}(), err: nil,
-		},
-		{
-			name:   "incorrect prev randao",
-			header: emptyPayloadHeader(),
-			err:    blocks.ErrInvalidPayloadPrevRandao,
-		},
-		{
-			name: "incorrect timestamp",
-			header: func() *enginev1.ExecutionPayloadHeader {
-				h := emptyPayloadHeader()
-				h.PrevRandao = random
-				h.Timestamp = 1
-				return h
-			}(),
-			err: blocks.ErrInvalidPayloadTimeStamp,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			wrappedHeader, err := wrapper.WrappedExecutionPayloadHeader(tt.header)
-			require.NoError(t, err)
-			err = blocks.ValidatePayloadHeader(st, wrappedHeader)
-			require.Equal(t, tt.err, err)
-		})
-	}
-}
-
 func Test_ValidatePayloadHeaderWhenMergeCompletes(t *testing.T) {
 	st, _ := util.DeterministicGenesisStateBellatrix(t, 1)
 	emptySt := st.Copy()

consensus-types/wrapper/BUILD.bazel

@@ -16,6 +16,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//config/fieldparams:go_default_library",
+        "//config/params:go_default_library",
         "//consensus-types/interfaces:go_default_library",
         "//consensus-types/primitives:go_default_library",
         "//encoding/bytesutil:go_default_library",
@@ -45,9 +46,11 @@ go_test(
     deps = [
         ":go_default_library",
         "//config/fieldparams:go_default_library",
+        "//config/params:go_default_library",
         "//consensus-types/interfaces:go_default_library",
         "//consensus-types/primitives:go_default_library",
         "//encoding/bytesutil:go_default_library",
+        "//encoding/ssz:go_default_library",
         "//proto/engine/v1:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
         "//proto/prysm/v1alpha1/validator-client:go_default_library",
@@ -55,6 +58,8 @@ go_test(
         "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
         "//testing/util:go_default_library",
+        "@com_github_ethereum_go_ethereum//common:go_default_library",
+        "@com_github_ethereum_go_ethereum//core/types:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
     ],
 )

consensus-types/wrapper/execution.go

@@ -6,6 +6,7 @@ import (
 	"github.com/pkg/errors"
 	fastssz "github.com/prysmaticlabs/fastssz"
 	fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
+	"github.com/prysmaticlabs/prysm/config/params"
 	"github.com/prysmaticlabs/prysm/consensus-types/interfaces"
 	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/encoding/ssz"
@@ -281,9 +282,12 @@ func PayloadToHeader(payload interfaces.ExecutionData) (*enginev1.ExecutionPaylo
 	if err != nil {
 		return nil, err
 	}
-	txRoot, err := ssz.TransactionsRoot(txs)
-	if err != nil {
-		return nil, err
+	txRoot := params.BeaconConfig().ZeroHash
+	if len(txs) > 0 {
+		txRoot, err = ssz.TransactionsRoot(txs)
+		if err != nil {
+			return nil, err
+		}
 	}
 	return &enginev1.ExecutionPayloadHeader{
 		ParentHash:       bytesutil.SafeCopyBytes(payload.ParentHash()),

consensus-types/wrapper/execution_test.go

@@ -1,16 +1,52 @@
 package wrapper_test
 
 import (
+	"math/big"
 	"testing"
 
+	"github.com/ethereum/go-ethereum/common"
+	gethtypes "github.com/ethereum/go-ethereum/core/types"
 	fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
+	"github.com/prysmaticlabs/prysm/config/params"
 	"github.com/prysmaticlabs/prysm/consensus-types/interfaces"
 	"github.com/prysmaticlabs/prysm/consensus-types/wrapper"
+	"github.com/prysmaticlabs/prysm/encoding/ssz"
 	enginev1 "github.com/prysmaticlabs/prysm/proto/engine/v1"
 	"github.com/prysmaticlabs/prysm/testing/assert"
 	"github.com/prysmaticlabs/prysm/testing/require"
 )
 
+func TestPayloadToHeader(t *testing.T) {
+	t.Run("no transactions should lead to a zero hash transactions root", func(t *testing.T) {
+		payload := createWrappedPayload(t)
+		wrapped, err := wrapper.PayloadToHeader(payload)
+		require.NoError(t, err)
+		require.DeepEqual(t, params.BeaconConfig().ZeroHash[:], wrapped.TransactionsRoot)
+	})
+	t.Run("transactions should lead to non-zero transactions root", func(t *testing.T) {
+		payload := createEmptyPayload()
+		tx := gethtypes.NewTransaction(
+			1,
+			common.BytesToAddress([]byte("hi")),
+			big.NewInt(0),
+			21000,
+			big.NewInt(1e6),
+			[]byte{},
+		)
+		enc, err := tx.MarshalBinary()
+		require.NoError(t, err)
+		payload.Transactions = [][]byte{enc}
+		txsRoot, err := ssz.TransactionsRoot(payload.Transactions)
+		require.NoError(t, err)
+		wrappedPayload, err := wrapper.WrappedExecutionPayload(payload)
+		require.NoError(t, err)
+		wrappedHeader, err := wrapper.PayloadToHeader(wrappedPayload)
+		require.NoError(t, err)
+		require.DeepNotEqual(t, params.BeaconConfig().ZeroHash, wrappedHeader.TransactionsRoot)
+		require.DeepEqual(t, txsRoot[:], wrappedHeader.TransactionsRoot)
+	})
+}
+
 func TestWrapExecutionPayload(t *testing.T) {
 	data := &enginev1.ExecutionPayload{GasUsed: 54}
 	wsb, err := wrapper.WrappedExecutionPayload(data)
@@ -81,8 +117,8 @@ func TestWrapExecutionPayloadHeader_SSZ(t *testing.T) {
 	assert.NoError(t, wsb.UnmarshalSSZ(encoded))
 }
 
-func createWrappedPayload(t testing.TB) interfaces.ExecutionData {
-	wsb, err := wrapper.WrappedExecutionPayload(&enginev1.ExecutionPayload{
+func createEmptyPayload() *enginev1.ExecutionPayload {
+	return &enginev1.ExecutionPayload{
 		ParentHash:    make([]byte, fieldparams.RootLength),
 		FeeRecipient:  make([]byte, fieldparams.FeeRecipientLength),
 		StateRoot:     make([]byte, fieldparams.RootLength),
@@ -97,7 +133,11 @@ func createWrappedPayload(t testing.TB) interfaces.ExecutionData {
 		BaseFeePerGas: make([]byte, fieldparams.RootLength),
 		BlockHash:     make([]byte, fieldparams.RootLength),
 		Transactions:  make([][]byte, 0),
-	})
+	}
+}
+
+func createWrappedPayload(t testing.TB) interfaces.ExecutionData {
+	wsb, err := wrapper.WrappedExecutionPayload(createEmptyPayload())
 	require.NoError(t, err)
 	return wsb
 }