-
Notifications
You must be signed in to change notification settings - Fork 253
Send getPayload request to all relays again #788
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didnt go a fine-grained code review, but agree with the high-level principle here that we query all relays
|
||
// Check if the relay supports SSZ | ||
relaySupportsSSZ := false | ||
for _, originalBidRelay := range originalBid.relays { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would be better as a simple map lookup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It cannot be a simple map lookup though. Because the SSZ field might be different.
|
||
// If the relay provided the bid in JSON or did not provide a bid for this payload, | ||
// we must convert the signed blinded beacon block from SSZ to JSON for this relay | ||
if parsedProposerContentType == MediaTypeOctetStream && !relaySupportsSSZ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not ideal to convert this again for every relay that would need it. would be better to check up-front if any relay will need it, and then prepare it once for all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. The conversion is quick. Let's just send out the SSZ requests as soon as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks okay, left some comments about possible improvements to the code. def preferable to submit getPayload to all relays.
📝 Summary
Instead of only sending the getPayload request to relays which provided this bid, send the getPayload request to all relays and convert the signed blinded beacon block to JSON if necessary.
⛱ Motivation and Context
In theory, this should improve liveness. If, for example, there is a relay which is geographically closes to the proposer, it might get the getPayload request sooner and therefore it would distribute the block faster.
📚 References
https://discord.com/channels/595666850260713488/1341434255154348034/1367203484620947508
✅ I have run these commands
make lint
make test-race
go mod tidy