From f9cc214c06037dfee6b767c33c33c11112d864e8 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 1 Oct 2024 12:37:38 +0200 Subject: [PATCH] eth/catalyst: change newPayload parameter to requestsHash --- beacon/engine/types.go | 14 +++----- eth/catalyst/api.go | 71 ++++++++++++++++------------------------ eth/catalyst/api_test.go | 18 +++------- 3 files changed, 37 insertions(+), 66 deletions(-) diff --git a/beacon/engine/types.go b/beacon/engine/types.go index 83a33c64b1..e6254f58f5 100644 --- a/beacon/engine/types.go +++ b/beacon/engine/types.go @@ -209,8 +209,8 @@ func decodeTransactions(enc [][]byte) ([]*types.Transaction, error) { // and that the blockhash of the constructed block matches the parameters. Nil // Withdrawals value will propagate through the returned block. Empty // Withdrawals value must be passed via non-nil, length 0 value in data. -func ExecutableDataToBlock(data ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requests [][]byte) (*types.Block, error) { - block, err := ExecutableDataToBlockNoHash(data, versionedHashes, beaconRoot, requests) +func ExecutableDataToBlock(data ExecutableData, versionedHashes []common.Hash, beaconRoot, requestsHash *common.Hash) (*types.Block, error) { + block, err := ExecutableDataToBlockNoHash(data, versionedHashes, beaconRoot, requestsHash) if err != nil { return nil, err } @@ -223,7 +223,7 @@ func ExecutableDataToBlock(data ExecutableData, versionedHashes []common.Hash, b // ExecutableDataToBlockNoHash is analogous to ExecutableDataToBlock, but is used // for stateless execution, so it skips checking if the executable data hashes to // the requested hash (stateless has to *compute* the root hash, it's not given). -func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requests [][]byte) (*types.Block, error) { +func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.Hash, beaconRoot, requestsHash *common.Hash) (*types.Block, error) { txs, err := decodeTransactions(data.Transactions) if err != nil { return nil, err @@ -258,12 +258,6 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H h := types.DeriveSha(types.Withdrawals(data.Withdrawals), trie.NewStackTrie(nil)) withdrawalsRoot = &h } - // Compute requestsHash if any requests were passed. - var requestsHash *common.Hash - if requests != nil { - h := types.CalcRequestsHash(requests) - requestsHash = &h - } header := &types.Header{ ParentHash: data.ParentHash, @@ -285,7 +279,7 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H ExcessBlobGas: data.ExcessBlobGas, BlobGasUsed: data.BlobGasUsed, ParentBeaconRoot: beaconRoot, - RequestsHash: requestsHash, // note: need to use recomputed hash value here to ensure validation. + RequestsHash: requestsHash, } return types.NewBlockWithHeader(header). WithBody(types.Body{Transactions: txs, Uncles: nil, Withdrawals: data.Withdrawals}). diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 864b1627f5..ef4534e943 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -541,7 +541,7 @@ func (api *ConsensusAPI) NewPayloadV1(params engine.ExecutableData) (engine.Payl if params.Withdrawals != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("withdrawals not supported in V1")) } - return api.newPayload(params, nil, nil, false) + return api.newPayload(params, nil, nil, nil, false) } // NewPayloadV2 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. @@ -564,7 +564,7 @@ func (api *ConsensusAPI) NewPayloadV2(params engine.ExecutableData) (engine.Payl if params.BlobGasUsed != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("non-nil blobGasUsed pre-cancun")) } - return api.newPayload(params, nil, nil, false) + return api.newPayload(params, nil, nil, nil, false) } // NewPayloadV3 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. @@ -589,12 +589,12 @@ func (api *ConsensusAPI) NewPayloadV3(params engine.ExecutableData, versionedHas if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Cancun { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.UnsupportedFork.With(errors.New("newPayloadV3 must only be called for cancun payloads")) } - return api.newPayload(params, versionedHashes, beaconRoot, false) + return api.newPayload(params, versionedHashes, beaconRoot, nil, false) } // NewPayloadV4 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. // NewPayloadV4 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. -func (api *ConsensusAPI) NewPayloadV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requests []hexutil.Bytes) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) NewPayloadV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot, requestsHash *common.Hash) (engine.PayloadStatusV1, error) { if params.Withdrawals == nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil withdrawals post-shanghai")) } @@ -611,14 +611,14 @@ func (api *ConsensusAPI) NewPayloadV4(params engine.ExecutableData, versionedHas if beaconRoot == nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil beaconRoot post-cancun")) } - if requests == nil { - return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil executionRequests post-cancun")) + if requestsHash == nil { + return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil executionRequestsHash post-prague")) } if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Prague { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.UnsupportedFork.With(errors.New("newPayloadV4 must only be called for prague payloads")) } - return api.newPayload(params, versionedHashes, beaconRoot, requests, false) + return api.newPayload(params, versionedHashes, beaconRoot, requestsHash, false) } // NewPayloadWithWitnessV1 is analogous to NewPayloadV1, only it also generates @@ -627,7 +627,7 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV1(params engine.ExecutableData) ( if params.Withdrawals != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("withdrawals not supported in V1")) } - return api.newPayload(params, nil, nil, true) + return api.newPayload(params, nil, nil, nil, true) } // NewPayloadWithWitnessV2 is analogous to NewPayloadV2, only it also generates @@ -651,7 +651,7 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV2(params engine.ExecutableData) ( if params.BlobGasUsed != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("non-nil blobGasUsed pre-cancun")) } - return api.newPayload(params, nil, nil, true) + return api.newPayload(params, nil, nil, nil, true) } // NewPayloadWithWitnessV3 is analogous to NewPayloadV3, only it also generates @@ -677,12 +677,12 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV3(params engine.ExecutableData, v if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Cancun { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.UnsupportedFork.With(errors.New("newPayloadWithWitnessV3 must only be called for cancun payloads")) } - return api.newPayload(params, versionedHashes, beaconRoot, true) + return api.newPayload(params, versionedHashes, beaconRoot, nil, true) } // NewPayloadWithWitnessV4 is analogous to NewPayloadV4, only it also generates // and returns a stateless witness after running the payload. -func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot, requestsHash *common.Hash) (engine.PayloadStatusV1, error) { if params.Withdrawals == nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil withdrawals post-shanghai")) } @@ -692,9 +692,6 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, v if params.BlobGasUsed == nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil blobGasUsed post-cancun")) } - if params.Requests == nil { - return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil requests post-prague")) - } if versionedHashes == nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil versionedHashes post-cancun")) @@ -702,11 +699,14 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, v if beaconRoot == nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil beaconRoot post-cancun")) } + if requestsHash == nil { + return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil executionRequestsHash post-prague")) + } if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Prague { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.UnsupportedFork.With(errors.New("newPayloadWithWitnessV4 must only be called for prague payloads")) } - return api.newPayload(params, versionedHashes, beaconRoot, true) + return api.newPayload(params, versionedHashes, beaconRoot, requestsHash, true) } // ExecuteStatelessPayloadV1 is analogous to NewPayloadV1, only it operates in @@ -715,7 +715,7 @@ func (api *ConsensusAPI) ExecuteStatelessPayloadV1(params engine.ExecutableData, if params.Withdrawals != nil { return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("withdrawals not supported in V1")) } - return api.executeStatelessPayload(params, nil, nil, opaqueWitness) + return api.executeStatelessPayload(params, nil, nil, nil, opaqueWitness) } // ExecuteStatelessPayloadV2 is analogous to NewPayloadV2, only it operates in @@ -739,7 +739,7 @@ func (api *ConsensusAPI) ExecuteStatelessPayloadV2(params engine.ExecutableData, if params.BlobGasUsed != nil { return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("non-nil blobGasUsed pre-cancun")) } - return api.executeStatelessPayload(params, nil, nil, opaqueWitness) + return api.executeStatelessPayload(params, nil, nil, nil, opaqueWitness) } // ExecuteStatelessPayloadV3 is analogous to NewPayloadV3, only it operates in @@ -765,12 +765,12 @@ func (api *ConsensusAPI) ExecuteStatelessPayloadV3(params engine.ExecutableData, if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Cancun { return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.UnsupportedFork.With(errors.New("executeStatelessPayloadV3 must only be called for cancun payloads")) } - return api.executeStatelessPayload(params, versionedHashes, beaconRoot, opaqueWitness) + return api.executeStatelessPayload(params, versionedHashes, beaconRoot, nil, opaqueWitness) } // ExecuteStatelessPayloadV4 is analogous to NewPayloadV4, only it operates in // a stateless mode on top of a provided witness instead of the local database. -func (api *ConsensusAPI) ExecuteStatelessPayloadV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requests []hexutil.Bytes, opaqueWitness hexutil.Bytes) (engine.StatelessPayloadStatusV1, error) { +func (api *ConsensusAPI) ExecuteStatelessPayloadV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requestsHash *common.Hash, opaqueWitness hexutil.Bytes) (engine.StatelessPayloadStatusV1, error) { if params.Withdrawals == nil { return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil withdrawals post-shanghai")) } @@ -787,17 +787,17 @@ func (api *ConsensusAPI) ExecuteStatelessPayloadV4(params engine.ExecutableData, if beaconRoot == nil { return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil beaconRoot post-cancun")) } - if requests == nil { - return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil executionRequests post-prague")) + if requestsHash == nil { + return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil executionRequestsHash post-prague")) } if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Prague { return engine.StatelessPayloadStatusV1{Status: engine.INVALID}, engine.UnsupportedFork.With(errors.New("executeStatelessPayloadV4 must only be called for prague payloads")) } - return api.executeStatelessPayload(params, versionedHashes, beaconRoot, requests, opaqueWitness) + return api.executeStatelessPayload(params, versionedHashes, beaconRoot, requestsHash, opaqueWitness) } -func (api *ConsensusAPI) newPayload(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requests []hexutil.Bytes, witness bool) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) newPayload(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot, requestsHash *common.Hash, witness bool) (engine.PayloadStatusV1, error) { // The locking here is, strictly, not required. Without these locks, this can happen: // // 1. NewPayload( execdata-N ) is invoked from the CL. It goes all the way down to @@ -815,7 +815,7 @@ func (api *ConsensusAPI) newPayload(params engine.ExecutableData, versionedHashe defer api.newPayloadLock.Unlock() log.Trace("Engine API request received", "method", "NewPayload", "number", params.Number, "hash", params.BlockHash) - block, err := engine.ExecutableDataToBlock(params, versionedHashes, beaconRoot, convertRequests(requests)) + block, err := engine.ExecutableDataToBlock(params, versionedHashes, beaconRoot, requestsHash) if err != nil { bgu := "nil" if params.BlobGasUsed != nil { @@ -843,7 +843,7 @@ func (api *ConsensusAPI) newPayload(params engine.ExecutableData, versionedHashe "len(params.Transactions)", len(params.Transactions), "len(params.Withdrawals)", len(params.Withdrawals), "beaconRoot", beaconRoot, - "len(requests)", len(requests), + "requestsHash", requestsHash, "error", err) return api.invalid(err, nil), nil } @@ -927,10 +927,10 @@ func (api *ConsensusAPI) newPayload(params engine.ExecutableData, versionedHashe return engine.PayloadStatusV1{Status: engine.VALID, Witness: ow, LatestValidHash: &hash}, nil } -func (api *ConsensusAPI) executeStatelessPayload(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, requests []hexutil.Bytes, opaqueWitness hexutil.Bytes) (engine.StatelessPayloadStatusV1, error) { +func (api *ConsensusAPI) executeStatelessPayload(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot, requestsHash *common.Hash, opaqueWitness hexutil.Bytes) (engine.StatelessPayloadStatusV1, error) { log.Trace("Engine API request received", "method", "ExecuteStatelessPayload", "number", params.Number, "hash", params.BlockHash) - block, err := engine.ExecutableDataToBlockNoHash(params, versionedHashes, beaconRoot, converRequests(requests)) + block, err := engine.ExecutableDataToBlockNoHash(params, versionedHashes, beaconRoot, requestsHash) if err != nil { bgu := "nil" if params.BlobGasUsed != nil { @@ -958,7 +958,7 @@ func (api *ConsensusAPI) executeStatelessPayload(params engine.ExecutableData, v "len(params.Transactions)", len(params.Transactions), "len(params.Withdrawals)", len(params.Withdrawals), "beaconRoot", beaconRoot, - "len(executionRequests)", len(requests), + "requestsHash", requestsHash, "error", err) errorMsg := err.Error() return engine.StatelessPayloadStatusV1{Status: engine.INVALID, ValidationError: &errorMsg}, nil @@ -1269,20 +1269,5 @@ func getBody(block *types.Block) *engine.ExecutionPayloadBody { result.Withdrawals = []*types.Withdrawal{} } - if reqs := block.Requests(); reqs != nil { - result.Requests = make([]hexutil.Bytes, len(reqs)) - for j, req := range reqs { - result.Requests[j] = req - } - } - return &result } - -func convertRequests(requests []hexutil.Bytes) [][]byte { - res := make([][]byte, len(requests)) - for i := range requests { - res[i] = requests[i] - } - return res -} diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 6a8c41a559..d3332e5142 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -25,7 +25,6 @@ import ( "math/big" "math/rand" "reflect" - "slices" "sync" "testing" "time" @@ -326,7 +325,7 @@ func TestEth2NewBlock(t *testing.T) { if err != nil { t.Fatalf("Failed to create the executable data, block %d: %v", i, err) } - block, err := engine.ExecutableDataToBlock(*execData, nil, nil) + block, err := engine.ExecutableDataToBlock(*execData, nil, nil, nil) if err != nil { t.Fatalf("Failed to convert executable data to block %v", err) } @@ -368,7 +367,7 @@ func TestEth2NewBlock(t *testing.T) { if err != nil { t.Fatalf("Failed to create the executable data %v", err) } - block, err := engine.ExecutableDataToBlock(*execData, nil, nil) + block, err := engine.ExecutableDataToBlock(*execData, nil, nil, nil) if err != nil { t.Fatalf("Failed to convert executable data to block %v", err) } @@ -509,7 +508,7 @@ func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.He } payload := getNewPayload(t, api, parent, w, h) - execResp, err := api.newPayload(*payload, []common.Hash{}, h, false) + execResp, err := api.newPayload(*payload, []common.Hash{}, h, nil, false) if err != nil { t.Fatalf("can't execute payload: %v", err) } @@ -1006,7 +1005,7 @@ func TestSimultaneousNewBlock(t *testing.T) { t.Fatal(testErr) } } - block, err := engine.ExecutableDataToBlock(*execData, nil, nil) + block, err := engine.ExecutableDataToBlock(*execData, nil, nil, nil) if err != nil { t.Fatalf("Failed to convert executable data to block %v", err) } @@ -1569,13 +1568,6 @@ func checkEqualBody(a *types.Body, b *engine.ExecutionPayloadBody) error { if !reflect.DeepEqual(a.Withdrawals, b.Withdrawals) { return fmt.Errorf("withdrawals mismatch") } - - reqEqual := slices.EqualFunc(a.Requests, b.Requests, func(a []byte, b hexutil.Bytes) bool { - return bytes.Equal(a, b) - }) - if !reqEqual { - return fmt.Errorf("requests mismatch") - } return nil } @@ -1611,7 +1603,7 @@ func TestBlockToPayloadWithBlobs(t *testing.T) { if got := len(envelope.BlobsBundle.Blobs); got != want { t.Fatalf("invalid number of blobs: got %v, want %v", got, want) } - _, err := engine.ExecutableDataToBlock(*envelope.ExecutionPayload, make([]common.Hash, 1), nil) + _, err := engine.ExecutableDataToBlock(*envelope.ExecutionPayload, make([]common.Hash, 1), nil, nil) if err != nil { t.Error(err) }