Faheelsattar/v2 get payload #2
Conversation
server/get_payload.go
Outdated
| } | ||
|
|
||
| // getPayloadV2 submits the signed blinded beacon block to relays for submission without returning the payload and blobs | ||
| func (m *BoostService) getPayloadV2(log *logrus.Entry, signedBlindedBeaconBlockBytes []byte, userAgent, proposerContentType, proposerAcceptContentTypes, proposerEthConsensusVersion string) (bool, bidResp) { |
There was a problem hiding this comment.
I think the only difference b/w getPayload and getPayloadV2 is the API call right? Can we avoid code duplication in this case and have a innerGetPayload method which takes in a parameter which decides whether to call a v1 api or v2 api? that would make the code much cleaner.
Sth like:
func (m *BoostService) innerGetPayload(log *logrus.Entry, signedBlindedBeaconBlockBytes []byte, userAgent, proposerContentType, proposerAcceptContentTypes, proposerEthConsensusVersion string, apiType string) (bool bidResp)
|
Hey @jtraglia would be great if you can also review it |
jtraglia
left a comment
jtraglia
left a comment
There was a problem hiding this comment.
Hey @faheelsattar, this looks really good. Overall I'm happy with the approach you took here. I just have a few little nits.
server/get_payload.go
Outdated
| // ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||
| // Core Logic | ||
| //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||
| // ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// |
There was a problem hiding this comment.
Why did we add a space to this section divider comments? Let's revert this.
There was a problem hiding this comment.
i think it was my formatter issue, fixed it
server/get_payload.go
Outdated
| PayloadV1 PayloadVersion = "V1" | ||
| PayloadV2 PayloadVersion = "V2" |
There was a problem hiding this comment.
PayloadV* is too ambiguous in my opinion. Makes me think the payload itself is changing. Let's rename these to GetPayloadV* & rename the type to GetPayloadVersion. Later, we can rename this to SubmitBlindedBlockV*.
server/get_payload.go
Outdated
| return <-resultCh, originalBid | ||
| result := <-resultCh | ||
| return result, originalBid |
There was a problem hiding this comment.
Why did you make this change? Let's revert if there's not a good reason.
server/get_payload.go
Outdated
| if version == GetPayloadV1 { | ||
| log.Debug("requesting payload") | ||
| } else { | ||
| log.Debug("requesting payload submission") | ||
| } |
There was a problem hiding this comment.
Could we replace this log message with
log.Debug("submitting signed blinded block")
This way we can avoid the if stmt.
server/get_payload.go
Outdated
| if version == GetPayloadV1 { | ||
| log.WithError(err).Warn("failed to get payload from relay after retries") | ||
| } else { | ||
| log.WithError(err).Warn("failed to request submit payload after retries") | ||
| } |
There was a problem hiding this comment.
can we do:
log.WithError(err).Warn("failed to submit signed blinded block after retries")
and avoid the if stmt.
server/get_payload.go
Outdated
| if version == GetPayloadV1 { | ||
| log.Info("received payload from relay") | ||
| } else { | ||
| log.Info("successfully submitted blinded block to relay") | ||
| } |
There was a problem hiding this comment.
I guess we could just do
log.Info("successfully submitted blinded block to relay)
|
looks fine overall to me! We should test this out locally on kurtosis |
3 participants
📝 Summary
Introduces v2 blinded blocks endpoint which doesn't return the entire
ExecutionPayloadandBlobsBundle⛱ Motivation and Context
Currently the
/eth/v1/builder/blinded_blocksreturns the entireExecutionPayloadandBlobsBundle. This can increase network bandwidth costs for the proposer and also the request time as we increase the number of Blobs. We therefore introduce a new v2 API endpoint,/eth/v2/builder/blinded_blockswhich no longer returns the fully signed unblinded payload. Instead, it returns an empty response and relies on the relay to propagate the block.📚 References
ethereum/builder-specs#119
ethereum/builder-specs#123
✅ I have run these commands
make lintmake test-racego mod tidy