From 10859e6d1b2dd5af9866a7ac817976e6726bf90b Mon Sep 17 00:00:00 2001 From: shana Date: Tue, 1 Aug 2023 17:06:25 -0700 Subject: [PATCH 1/2] Switch to publishBlockV2 endpoint (#492) --- beaconclient/mock_beacon_instance.go | 2 +- beaconclient/multi_beacon_client.go | 30 ++++++++++++++++++++++++-- beaconclient/prod_beacon_instance.go | 32 +++++++++++++++------------- beaconclient/util.go | 16 +++++++++++++- 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/beaconclient/mock_beacon_instance.go b/beaconclient/mock_beacon_instance.go index 1b36faf8..95902de5 100644 --- a/beaconclient/mock_beacon_instance.go +++ b/beaconclient/mock_beacon_instance.go @@ -107,7 +107,7 @@ func (c *MockBeaconInstance) addDelay() { } } -func (c *MockBeaconInstance) PublishBlock(block *common.SignedBeaconBlock) (code int, err error) { +func (c *MockBeaconInstance) PublishBlock(block *common.SignedBeaconBlock, broadcastValidation BroadcastValidation) (code int, err error) { return 0, nil } diff --git a/beaconclient/multi_beacon_client.go b/beaconclient/multi_beacon_client.go index 5ae80969..7d731022 100644 --- a/beaconclient/multi_beacon_client.go +++ b/beaconclient/multi_beacon_client.go @@ -20,6 +20,18 @@ var ( ErrBeaconBlock202 = errors.New("beacon block failed validation but was still broadcast (202)") ) +type BroadcastValidation int + +const ( + Gossip BroadcastValidation = iota // lightweight gossip checks only + Consensus // full consensus checks, including validation of all signatures and blocks fields + ConsensusAndEquivocation // the same as `consensus`, with an extra equivocation check +) + +func (b BroadcastValidation) String() string { + return [...]string{"gossip", "consensus", "consensus_and_equivocation"}[b] +} + // IMultiBeaconClient is the interface for the MultiBeaconClient, which can manage several beacon client instances under the hood type IMultiBeaconClient interface { BestSyncStatus() (*SyncStatusPayloadData, error) @@ -48,7 +60,7 @@ type IBeaconInstance interface { GetStateValidators(stateID string) (*GetStateValidatorsResponse, error) GetProposerDuties(epoch uint64) (*ProposerDutiesResponse, error) GetURI() string - PublishBlock(block *common.SignedBeaconBlock) (code int, err error) + PublishBlock(block *common.SignedBeaconBlock, broadcastValidation BroadcastValidation) (code int, err error) GetGenesis() (*GetGenesisResponse, error) GetSpec() (spec *GetSpecResponse, err error) GetForkSchedule() (spec *GetForkScheduleResponse, err error) @@ -64,6 +76,7 @@ type MultiBeaconClient struct { // feature flags ffAllowSyncingBeaconNode bool + ffBroadcastValidation BroadcastValidation } func NewMultiBeaconClient(log *logrus.Entry, beaconInstances []IBeaconInstance) *MultiBeaconClient { @@ -72,6 +85,7 @@ func NewMultiBeaconClient(log *logrus.Entry, beaconInstances []IBeaconInstance) beaconInstances: beaconInstances, bestBeaconIndex: *uberatomic.NewInt64(0), ffAllowSyncingBeaconNode: false, + ffBroadcastValidation: ConsensusAndEquivocation, } // feature flags @@ -80,6 +94,18 @@ func NewMultiBeaconClient(log *logrus.Entry, beaconInstances []IBeaconInstance) client.ffAllowSyncingBeaconNode = true } + if os.Getenv("BROADCAST_VALIDATION") != "" { + broadcastValidationStr := os.Getenv("BROADCAST_VALIDATION") + broadcastValidation, ok := parseBroadcastValidationString(broadcastValidationStr) + if !ok { + msg := fmt.Sprintf("env: BROADCAST_VALIDATION: invalid value %s, leaving to default value %s", broadcastValidationStr, client.ffBroadcastValidation.String()) + client.log.Warn(msg) + } else { + client.log.Info(fmt.Sprintf("env: BROADCAST_VALIDATION: setting validation to %s", broadcastValidation.String())) + client.ffBroadcastValidation = broadcastValidation + } + } + return client } @@ -243,7 +269,7 @@ func (c *MultiBeaconClient) PublishBlock(block *common.SignedBeaconBlock) (code log := log.WithField("uri", client.GetURI()) log.Debug("publishing block") go func(index int, client IBeaconInstance) { - code, err := client.PublishBlock(block) + code, err := client.PublishBlock(block, c.ffBroadcastValidation) resChans <- publishResp{ index: index, code: code, diff --git a/beaconclient/prod_beacon_instance.go b/beaconclient/prod_beacon_instance.go index f491d4da..441eb0a9 100644 --- a/beaconclient/prod_beacon_instance.go +++ b/beaconclient/prod_beacon_instance.go @@ -138,7 +138,7 @@ type ValidatorResponseValidatorData struct { func (c *ProdBeaconInstance) GetStateValidators(stateID string) (*GetStateValidatorsResponse, error) { uri := fmt.Sprintf("%s/eth/v1/beacon/states/%s/validators?status=active,pending", c.beaconURI, stateID) vd := new(GetStateValidatorsResponse) - _, err := fetchBeacon(http.MethodGet, uri, nil, vd, nil) + _, err := fetchBeacon(http.MethodGet, uri, nil, vd, nil, http.Header{}) return vd, err } @@ -159,7 +159,7 @@ func (c *ProdBeaconInstance) SyncStatus() (*SyncStatusPayloadData, error) { uri := c.beaconURI + "/eth/v1/node/syncing" timeout := 5 * time.Second resp := new(SyncStatusPayload) - _, err := fetchBeacon(http.MethodGet, uri, nil, resp, &timeout) + _, err := fetchBeacon(http.MethodGet, uri, nil, resp, &timeout, http.Header{}) if err != nil { return nil, err } @@ -189,7 +189,7 @@ type ProposerDutiesResponseData struct { func (c *ProdBeaconInstance) GetProposerDuties(epoch uint64) (*ProposerDutiesResponse, error) { uri := fmt.Sprintf("%s/eth/v1/validator/duties/proposer/%d", c.beaconURI, epoch) resp := new(ProposerDutiesResponse) - _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -212,7 +212,7 @@ type GetHeaderResponseMessage struct { func (c *ProdBeaconInstance) GetHeader() (*GetHeaderResponse, error) { uri := fmt.Sprintf("%s/eth/v1/beacon/headers/head", c.beaconURI) resp := new(GetHeaderResponse) - _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -220,7 +220,7 @@ func (c *ProdBeaconInstance) GetHeader() (*GetHeaderResponse, error) { func (c *ProdBeaconInstance) GetHeaderForSlot(slot uint64) (*GetHeaderResponse, error) { uri := fmt.Sprintf("%s/eth/v1/beacon/headers/%d", c.beaconURI, slot) resp := new(GetHeaderResponse) - _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -240,7 +240,7 @@ type GetBlockResponse struct { func (c *ProdBeaconInstance) GetBlock(blockID string) (block *GetBlockResponse, err error) { uri := fmt.Sprintf("%s/eth/v2/beacon/blocks/%s", c.beaconURI, blockID) resp := new(GetBlockResponse) - _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -248,7 +248,7 @@ func (c *ProdBeaconInstance) GetBlock(blockID string) (block *GetBlockResponse, func (c *ProdBeaconInstance) GetBlockForSlot(slot uint64) (*GetBlockResponse, error) { uri := fmt.Sprintf("%s/eth/v2/beacon/blocks/%d", c.beaconURI, slot) resp := new(GetBlockResponse) - _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -256,9 +256,11 @@ func (c *ProdBeaconInstance) GetURI() string { return c.beaconURI } -func (c *ProdBeaconInstance) PublishBlock(block *common.SignedBeaconBlock) (code int, err error) { - uri := fmt.Sprintf("%s/eth/v1/beacon/blocks", c.beaconURI) - return fetchBeacon(http.MethodPost, uri, block, nil, nil) +func (c *ProdBeaconInstance) PublishBlock(block *common.SignedBeaconBlock, broadcastValidation BroadcastValidation) (code int, err error) { + uri := fmt.Sprintf("%s/eth/v2/beacon/blocks?broadcast_validation=%s", c.beaconURI, broadcastValidation.String()) + headers := http.Header{} + headers.Add("Eth-Consensus-Version", common.ForkVersionStringCapella) + return fetchBeacon(http.MethodPost, uri, block, nil, nil, headers) } type GetGenesisResponse struct { @@ -275,7 +277,7 @@ type GetGenesisResponseData struct { func (c *ProdBeaconInstance) GetGenesis() (*GetGenesisResponse, error) { uri := fmt.Sprintf("%s/eth/v1/beacon/genesis", c.beaconURI) resp := new(GetGenesisResponse) - _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err := fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -292,7 +294,7 @@ type GetSpecResponse struct { func (c *ProdBeaconInstance) GetSpec() (spec *GetSpecResponse, err error) { uri := fmt.Sprintf("%s/eth/v1/config/spec", c.beaconURI) resp := new(GetSpecResponse) - _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -308,7 +310,7 @@ type GetForkScheduleResponse struct { func (c *ProdBeaconInstance) GetForkSchedule() (spec *GetForkScheduleResponse, err error) { uri := fmt.Sprintf("%s/eth/v1/config/fork_schedule", c.beaconURI) resp := new(GetForkScheduleResponse) - _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -322,7 +324,7 @@ type GetRandaoResponse struct { func (c *ProdBeaconInstance) GetRandao(slot uint64) (randaoResp *GetRandaoResponse, err error) { uri := fmt.Sprintf("%s/eth/v1/beacon/states/%d/randao", c.beaconURI, slot) resp := new(GetRandaoResponse) - _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } @@ -336,6 +338,6 @@ type GetWithdrawalsResponse struct { func (c *ProdBeaconInstance) GetWithdrawals(slot uint64) (withdrawalsResp *GetWithdrawalsResponse, err error) { uri := fmt.Sprintf("%s/eth/v1/beacon/states/%d/withdrawals", c.beaconURI, slot) resp := new(GetWithdrawalsResponse) - _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil) + _, err = fetchBeacon(http.MethodGet, uri, nil, resp, nil, http.Header{}) return resp, err } diff --git a/beaconclient/util.go b/beaconclient/util.go index bb062de8..6d2fd289 100644 --- a/beaconclient/util.go +++ b/beaconclient/util.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "strings" "time" ) @@ -19,7 +20,17 @@ var ( StateIDJustified = "justified" ) -func fetchBeacon(method, url string, payload, dst any, timeout *time.Duration) (code int, err error) { +func parseBroadcastValidationString(s string) (BroadcastValidation, bool) { + broadcastValidationMap := map[string]BroadcastValidation{ + "gossip": Gossip, + "consensus": Consensus, + "consensus_and_equivocation": ConsensusAndEquivocation, + } + b, ok := broadcastValidationMap[strings.ToLower(s)] + return b, ok +} + +func fetchBeacon(method, url string, payload, dst any, timeout *time.Duration, headers http.Header) (code int, err error) { var req *http.Request if payload == nil { @@ -33,6 +44,9 @@ func fetchBeacon(method, url string, payload, dst any, timeout *time.Duration) ( // Set content-type req.Header.Add("Content-Type", "application/json") + for k, v := range headers { + req.Header.Add(k, v[0]) + } } if err != nil { From 2944f4f1283b32f655eabc6a2de36e08c74ec932 Mon Sep 17 00:00:00 2001 From: mike neuder Date: Wed, 2 Aug 2023 08:45:47 -0400 Subject: [PATCH 2/2] [code health] refactor payload attrs checks in handleSubmitNewBlock (#491) refactor payload attrs checks to isolated function --- services/api/service.go | 67 ++++++++------- services/api/service_test.go | 153 ++++++++++++++++++++++++++++++++++- 2 files changed, 189 insertions(+), 31 deletions(-) diff --git a/services/api/service.go b/services/api/service.go index 8a58c88d..1ebace74 100644 --- a/services/api/service.go +++ b/services/api/service.go @@ -1502,6 +1502,42 @@ func (api *RelayAPI) checkSubmissionFeeRecipient(w http.ResponseWriter, log *log return slotDuty.Entry.Message.GasLimit, true } +func (api *RelayAPI) checkSubmissionPayloadAttrs(w http.ResponseWriter, log *logrus.Entry, payload *common.BuilderSubmitBlockRequest) bool { + api.payloadAttributesLock.RLock() + attrs, ok := api.payloadAttributes[payload.ParentHash()] + api.payloadAttributesLock.RUnlock() + if !ok || payload.Slot() != attrs.slot { + log.Warn("payload attributes not (yet) known") + api.RespondError(w, http.StatusBadRequest, "payload attributes not (yet) known") + return false + } + + if payload.Random() != attrs.payloadAttributes.PrevRandao { + msg := fmt.Sprintf("incorrect prev_randao - got: %s, expected: %s", payload.Random(), attrs.payloadAttributes.PrevRandao) + log.Info(msg) + api.RespondError(w, http.StatusBadRequest, msg) + return false + } + + if hasReachedFork(payload.Slot(), api.capellaEpoch) { // Capella requires correct withdrawals + withdrawalsRoot, err := ComputeWithdrawalsRoot(payload.Withdrawals()) + if err != nil { + log.WithError(err).Warn("could not compute withdrawals root from payload") + api.RespondError(w, http.StatusBadRequest, "could not compute withdrawals root") + return false + } + + if withdrawalsRoot != attrs.withdrawalsRoot { + msg := fmt.Sprintf("incorrect withdrawals root - got: %s, expected: %s", withdrawalsRoot.String(), attrs.withdrawalsRoot.String()) + log.Info(msg) + api.RespondError(w, http.StatusBadRequest, msg) + return false + } + } + + return true +} + func (api *RelayAPI) handleSubmitNewBlock(w http.ResponseWriter, req *http.Request) { //nolint:gocognit,maintidx var pf common.Profile var prevTime, nextTime time.Time @@ -1685,38 +1721,11 @@ func (api *RelayAPI) handleSubmitNewBlock(w http.ResponseWriter, req *http.Reque log = log.WithField("timestampBeforeAttributesCheck", time.Now().UTC().UnixMilli()) - api.payloadAttributesLock.RLock() - attrs, ok := api.payloadAttributes[payload.ParentHash()] - api.payloadAttributesLock.RUnlock() - if !ok || payload.Slot() != attrs.slot { - log.Warn("payload attributes not (yet) known") - api.RespondError(w, http.StatusBadRequest, "payload attributes not (yet) known") - return - } - - if payload.Random() != attrs.payloadAttributes.PrevRandao { - msg := fmt.Sprintf("incorrect prev_randao - got: %s, expected: %s", payload.Random(), attrs.payloadAttributes.PrevRandao) - log.Info(msg) - api.RespondError(w, http.StatusBadRequest, msg) + cont = api.checkSubmissionPayloadAttrs(w, log, payload) + if !cont { return } - if hasReachedFork(payload.Slot(), api.capellaEpoch) { // Capella requires correct withdrawals - withdrawalsRoot, err := ComputeWithdrawalsRoot(payload.Withdrawals()) - if err != nil { - log.WithError(err).Warn("could not compute withdrawals root from payload") - api.RespondError(w, http.StatusBadRequest, "could not compute withdrawals root") - return - } - - if withdrawalsRoot != attrs.withdrawalsRoot { - msg := fmt.Sprintf("incorrect withdrawals root - got: %s, expected: %s", withdrawalsRoot.String(), attrs.withdrawalsRoot.String()) - log.Info(msg) - api.RespondError(w, http.StatusBadRequest, msg) - return - } - } - // Verify the signature log = log.WithField("timestampBeforeSignatureCheck", time.Now().UTC().UnixMilli()) signature := payload.Signature() diff --git a/services/api/service_test.go b/services/api/service_test.go index a23dccb9..882a0015 100644 --- a/services/api/service_test.go +++ b/services/api/service_test.go @@ -16,6 +16,7 @@ import ( builderCapella "github.com/attestantio/go-builder-client/api/capella" v1 "github.com/attestantio/go-builder-client/api/v1" "github.com/attestantio/go-eth2-client/spec/bellatrix" + "github.com/attestantio/go-eth2-client/spec/capella" "github.com/attestantio/go-eth2-client/spec/phase0" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/flashbots/go-boost-utils/bls" @@ -30,8 +31,11 @@ import ( ) const ( - testGasLimit = uint64(30000000) - testSlot = uint64(42) + testGasLimit = uint64(30000000) + testSlot = uint64(42) + testParentHash = "0xbd3291854dc822b7ec585925cda0e18f06af28fa2886e15f52d52dd4b6f94ed6" + testWithdrawalsRoot = "0x7f6d156912a4cb1e74ee37e492ad883f7f7ac856d987b3228b517e490aa0189e" + testPrevRandao = "0x9962816e9d0a39fd4c80935338a741dc916d1545694e41eb5a505e1a3098f9e4" ) var ( @@ -580,6 +584,151 @@ func TestCheckSubmissionFeeRecipient(t *testing.T) { } } +func TestCheckSubmissionPayloadAttrs(t *testing.T) { + withdrawalsRoot, err := hexutil.Decode(testWithdrawalsRoot) + require.NoError(t, err) + prevRandao, err := hexutil.Decode(testPrevRandao) + require.NoError(t, err) + parentHash, err := hexutil.Decode(testParentHash) + require.NoError(t, err) + + cases := []struct { + description string + attrs payloadAttributesHelper + payload *common.BuilderSubmitBlockRequest + expectCont bool + }{ + { + description: "success", + attrs: payloadAttributesHelper{ + slot: testSlot, + parentHash: testParentHash, + withdrawalsRoot: phase0.Root(withdrawalsRoot), + payloadAttributes: beaconclient.PayloadAttributes{ + PrevRandao: testPrevRandao, + }, + }, + payload: &common.BuilderSubmitBlockRequest{ + Capella: &builderCapella.SubmitBlockRequest{ + ExecutionPayload: &capella.ExecutionPayload{ + PrevRandao: [32]byte(prevRandao), + Withdrawals: []*capella.Withdrawal{ + { + Index: 989694, + }, + }, + }, + Message: &v1.BidTrace{ + Slot: testSlot, + ParentHash: phase0.Hash32(parentHash), + }, + }, + }, + expectCont: true, + }, + { + description: "failure_attrs_not_known", + attrs: payloadAttributesHelper{ + slot: testSlot, + }, + payload: &common.BuilderSubmitBlockRequest{ + Capella: &builderCapella.SubmitBlockRequest{ + Message: &v1.BidTrace{ + Slot: testSlot + 1, // submission for a future slot + }, + }, + }, + expectCont: false, + }, + { + description: "failure_wrong_prev_randao", + attrs: payloadAttributesHelper{ + slot: testSlot, + payloadAttributes: beaconclient.PayloadAttributes{ + PrevRandao: testPrevRandao, + }, + }, + payload: &common.BuilderSubmitBlockRequest{ + Capella: &builderCapella.SubmitBlockRequest{ + Message: &v1.BidTrace{ + Slot: testSlot, + ParentHash: phase0.Hash32(parentHash), + }, + ExecutionPayload: &capella.ExecutionPayload{ + PrevRandao: [32]byte(parentHash), // use a different hash to cause an error + }, + }, + }, + expectCont: false, + }, + { + description: "failure_nil_withdrawals", + attrs: payloadAttributesHelper{ + slot: testSlot, + payloadAttributes: beaconclient.PayloadAttributes{ + PrevRandao: testPrevRandao, + }, + }, + payload: &common.BuilderSubmitBlockRequest{ + Capella: &builderCapella.SubmitBlockRequest{ + Message: &v1.BidTrace{ + Slot: testSlot, + ParentHash: phase0.Hash32(parentHash), + }, + ExecutionPayload: &capella.ExecutionPayload{ + PrevRandao: [32]byte(prevRandao), + Withdrawals: nil, // set to nil to cause an error + }, + }, + }, + expectCont: false, + }, + { + description: "failure_wrong_withdrawal_root", + attrs: payloadAttributesHelper{ + slot: testSlot, + parentHash: testParentHash, + withdrawalsRoot: phase0.Root(prevRandao), // use different root to cause an error + payloadAttributes: beaconclient.PayloadAttributes{ + PrevRandao: testPrevRandao, + }, + }, + payload: &common.BuilderSubmitBlockRequest{ + Capella: &builderCapella.SubmitBlockRequest{ + ExecutionPayload: &capella.ExecutionPayload{ + PrevRandao: [32]byte(prevRandao), + Withdrawals: []*capella.Withdrawal{ + { + Index: 989694, + }, + }, + }, + Message: &v1.BidTrace{ + Slot: testSlot, + ParentHash: phase0.Hash32(parentHash), + }, + }, + }, + expectCont: false, + }, + } + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + _, _, backend := startTestBackend(t) + backend.relay.payloadAttributesLock.RLock() + backend.relay.payloadAttributes[testParentHash] = tc.attrs + backend.relay.payloadAttributesLock.RUnlock() + backend.relay.capellaEpoch = 1 + + w := httptest.NewRecorder() + logger := logrus.New() + log := logrus.NewEntry(logger) + cont := backend.relay.checkSubmissionPayloadAttrs(w, log, tc.payload) + require.Equal(t, tc.expectCont, cont) + }) + } +} + func gzipBytes(t *testing.T, b []byte) []byte { t.Helper() var buf bytes.Buffer