Skip to content

Commit

Permalink
beacon/engine: don't omit empty withdrawals in ExecutionPayloadBodies (…
Browse files Browse the repository at this point in the history 
…#26698)

This ensures the "withdrawals" field will always be present in responses
to getPayloadBodiesByRangeV1 and getPayloadBodiesByHashV1.

---------

Co-authored-by: Felix Lange <[email protected]>
  • Loading branch information
@MariusVanDerWijden @fjl
MariusVanDerWijden and fjl authored Mar 7, 2023
1 parent 1e3177d commit 78429f7
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 33 deletions.
2 changes: 1 addition & 1 deletion beacon/engine/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,5 +233,5 @@ func BlockToExecutableData(block *types.Block, fees *big.Int) *ExecutionPayloadE
// ExecutionPayloadBodyV1 is used in the response to GetPayloadBodiesByHashV1 and GetPayloadBodiesByRangeV1
type ExecutionPayloadBodyV1 struct {
TransactionData []hexutil.Bytes `json:"transactions"`
Withdrawals []*types.Withdrawal `json:"withdrawals,omitempty"`
Withdrawals []*types.Withdrawal `json:"withdrawals"`
}
108 changes: 76 additions & 32 deletions eth/catalyst/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ package catalyst
import (
"bytes"
"context"
crand "crypto/rand"
"fmt"
"math/big"
"math/rand"
"reflect"
"sync"
"testing"
"time"
Expand All @@ -41,7 +44,6 @@ import (
"github.com/ethereum/go-ethereum/node"
"github.com/ethereum/go-ethereum/p2p"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/rpc"
"github.com/ethereum/go-ethereum/trie"
)
Expand Down Expand Up @@ -473,18 +475,21 @@ func TestFullAPI(t *testing.T) {
ethservice.TxPool().AddLocal(tx)
}

setupBlocks(t, ethservice, 10, parent, callback)
setupBlocks(t, ethservice, 10, parent, callback, nil)
}

func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.Header, callback func(parent *types.Header)) []*types.Header {
func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.Header, callback func(parent *types.Header), withdrawals [][]*types.Withdrawal) []*types.Header {
api := NewConsensusAPI(ethservice)
var blocks []*types.Header
for i := 0; i < n; i++ {
callback(parent)
var w []*types.Withdrawal
if withdrawals != nil {
w = withdrawals[i]
}

payload := getNewPayload(t, api, parent)

execResp, err := api.NewPayloadV1(*payload)
payload := getNewPayload(t, api, parent, w)
execResp, err := api.NewPayloadV2(*payload)
if err != nil {
t.Fatalf("can't execute payload: %v", err)
}
Expand Down Expand Up @@ -676,10 +681,10 @@ func TestEmptyBlocks(t *testing.T) {
api := NewConsensusAPI(ethservice)

// Setup 10 blocks on the canonical chain
setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Header) {})
setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Header) {}, nil)

// (1) check LatestValidHash by sending a normal payload (P1'')
payload := getNewPayload(t, api, commonAncestor)
payload := getNewPayload(t, api, commonAncestor, nil)

status, err := api.NewPayloadV1(*payload)
if err != nil {
Expand All @@ -693,7 +698,7 @@ func TestEmptyBlocks(t *testing.T) {
}

// (2) Now send P1' which is invalid
payload = getNewPayload(t, api, commonAncestor)
payload = getNewPayload(t, api, commonAncestor, nil)
payload.GasUsed += 1
payload = setBlockhash(payload)
// Now latestValidHash should be the common ancestor
Expand All @@ -711,7 +716,7 @@ func TestEmptyBlocks(t *testing.T) {
}

// (3) Now send a payload with unknown parent
payload = getNewPayload(t, api, commonAncestor)
payload = getNewPayload(t, api, commonAncestor, nil)
payload.ParentHash = common.Hash{1}
payload = setBlockhash(payload)
// Now latestValidHash should be the common ancestor
Expand All @@ -727,11 +732,12 @@ func TestEmptyBlocks(t *testing.T) {
}
}

func getNewPayload(t *testing.T, api *ConsensusAPI, parent *types.Header) *engine.ExecutableData {
func getNewPayload(t *testing.T, api *ConsensusAPI, parent *types.Header, withdrawals []*types.Withdrawal) *engine.ExecutableData {
params := engine.PayloadAttributes{
Timestamp: parent.Time + 1,
Random: crypto.Keccak256Hash([]byte{byte(1)}),
SuggestedFeeRecipient: parent.Coinbase,
Withdrawals: withdrawals,
}

payload, err := assembleBlock(api, parent.Hash(), &params)
Expand Down Expand Up @@ -799,7 +805,7 @@ func TestTrickRemoteBlockCache(t *testing.T) {
commonAncestor := ethserviceA.BlockChain().CurrentBlock()

// Setup 10 blocks on the canonical chain
setupBlocks(t, ethserviceA, 10, commonAncestor, func(parent *types.Header) {})
setupBlocks(t, ethserviceA, 10, commonAncestor, func(parent *types.Header) {}, nil)
commonAncestor = ethserviceA.BlockChain().CurrentBlock()

var invalidChain []*engine.ExecutableData
Expand All @@ -808,7 +814,7 @@ func TestTrickRemoteBlockCache(t *testing.T) {
//invalidChain = append(invalidChain, payload1)

// create an invalid payload2 (P2)
payload2 := getNewPayload(t, apiA, commonAncestor)
payload2 := getNewPayload(t, apiA, commonAncestor, nil)
//payload2.ParentHash = payload1.BlockHash
payload2.GasUsed += 1
payload2 = setBlockhash(payload2)
Expand All @@ -817,7 +823,7 @@ func TestTrickRemoteBlockCache(t *testing.T) {
head := payload2
// create some valid payloads on top
for i := 0; i < 10; i++ {
payload := getNewPayload(t, apiA, commonAncestor)
payload := getNewPayload(t, apiA, commonAncestor, nil)
payload.ParentHash = head.BlockHash
payload = setBlockhash(payload)
invalidChain = append(invalidChain, payload)
Expand Down Expand Up @@ -855,10 +861,10 @@ func TestInvalidBloom(t *testing.T) {
api := NewConsensusAPI(ethservice)

// Setup 10 blocks on the canonical chain
setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Header) {})
setupBlocks(t, ethservice, 10, commonAncestor, func(parent *types.Header) {}, nil)

// (1) check LatestValidHash by sending a normal payload (P1'')
payload := getNewPayload(t, api, commonAncestor)
payload := getNewPayload(t, api, commonAncestor, nil)
payload.LogsBloom = append(payload.LogsBloom, byte(1))
status, err := api.NewPayloadV1(*payload)
if err != nil {
Expand Down Expand Up @@ -1233,8 +1239,10 @@ func TestNilWithdrawals(t *testing.T) {
}

func setupBodies(t *testing.T) (*node.Node, *eth.Ethereum, []*types.Block) {
genesis, preMergeBlocks := generateMergeChain(10, false)
n, ethservice := startEthService(t, genesis, preMergeBlocks)
genesis, blocks := generateMergeChain(10, true)
n, ethservice := startEthService(t, genesis, blocks)
// enable shanghai on the last block
ethservice.BlockChain().Config().ShanghaiTime = &blocks[len(blocks)-1].Header().Time

var (
parent = ethservice.BlockChain().CurrentBlock()
Expand All @@ -1249,12 +1257,38 @@ func setupBodies(t *testing.T) (*node.Node, *eth.Ethereum, []*types.Block) {
ethservice.TxPool().AddLocal(tx)
}

postMergeHeaders := setupBlocks(t, ethservice, 10, parent, callback)
postMergeBlocks := make([]*types.Block, len(postMergeHeaders))
for i, header := range postMergeHeaders {
postMergeBlocks[i] = ethservice.BlockChain().GetBlock(header.Hash(), header.Number.Uint64())
withdrawals := make([][]*types.Withdrawal, 10)
withdrawals[0] = nil // should be filtered out by miner
withdrawals[1] = make([]*types.Withdrawal, 0)
for i := 2; i < len(withdrawals); i++ {
addr := make([]byte, 20)
crand.Read(addr)
withdrawals[i] = []*types.Withdrawal{
{Index: rand.Uint64(), Validator: rand.Uint64(), Amount: rand.Uint64(), Address: common.BytesToAddress(addr)},
}
}

postShanghaiHeaders := setupBlocks(t, ethservice, 10, parent, callback, withdrawals)
postShanghaiBlocks := make([]*types.Block, len(postShanghaiHeaders))
for i, header := range postShanghaiHeaders {
postShanghaiBlocks[i] = ethservice.BlockChain().GetBlock(header.Hash(), header.Number.Uint64())
}
return n, ethservice, append(preMergeBlocks, postMergeBlocks...)
return n, ethservice, append(blocks, postShanghaiBlocks...)
}

func allHashes(blocks []*types.Block) []common.Hash {
var hashes []common.Hash
for _, b := range blocks {
hashes = append(hashes, b.Hash())
}
return hashes
}
func allBodies(blocks []*types.Block) []*types.Body {
var bodies []*types.Body
for _, b := range blocks {
bodies = append(bodies, b.Body())
}
return bodies
}

func TestGetBlockBodiesByHash(t *testing.T) {
Expand Down Expand Up @@ -1296,6 +1330,11 @@ func TestGetBlockBodiesByHash(t *testing.T) {
results: []*types.Body{blocks[0].Body(), nil, blocks[0].Body(), blocks[0].Body()},
hashes: []common.Hash{blocks[0].Hash(), {1, 2}, blocks[0].Hash(), blocks[0].Hash()},
},
// all blocks
{
results: allBodies(blocks),
hashes: allHashes(blocks),
},
}

for k, test := range tests {
Expand Down Expand Up @@ -1364,6 +1403,12 @@ func TestGetBlockBodiesByRange(t *testing.T) {
start: 22,
count: 2,
},
// allBlocks
{
results: allBodies(blocks),
start: 1,
count: hexutil.Uint64(len(blocks)),
},
}

for k, test := range tests {
Expand Down Expand Up @@ -1434,15 +1479,14 @@ func equalBody(a *types.Body, b *engine.ExecutionPayloadBodyV1) bool {
} else if a == nil || b == nil {
return false
}
var want []hexutil.Bytes
for _, tx := range a.Transactions {
data, _ := tx.MarshalBinary()
want = append(want, hexutil.Bytes(data))
}
aBytes, errA := rlp.EncodeToBytes(want)
bBytes, errB := rlp.EncodeToBytes(b.TransactionData)
if errA != errB {
if len(a.Transactions) != len(b.TransactionData) {
return false
}
return bytes.Equal(aBytes, bBytes)
for i, tx := range a.Transactions {
data, _ := tx.MarshalBinary()
if !bytes.Equal(data, b.TransactionData[i]) {
return false
}
}
return reflect.DeepEqual(a.Withdrawals, b.Withdrawals)
}

0 comments on commit 78429f7

Please sign in to comment.