Skip to content
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

Allow SSZ builder submissions (also SSZ+GZIP) #388

Merged
merged 7 commits into from
May 8, 2023

Conversation

metachris
Copy link
Collaborator

📝 Summary

Allows block builder submissions also in SSZ, besides JSON.

Encoding Bytes
JSON 704810
SSZ 352239
JSON+GZIP 207788
SSZ+GZIP 195923

Decoding performance

BenchmarkDecoding/json
BenchmarkDecoding/json-10                  18710             63206 ns/op            5712 B/op         75 allocs/op
BenchmarkDecoding/ssz
BenchmarkDecoding/ssz-10                 5619360               224.4 ns/op           185 B/op          0 allocs/op

✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy
  • I have seen and agree to CONTRIBUTING.md

@metachris metachris force-pushed the ssz-builder-submission branch 2 times, most recently from da5dd20 to 35b6f9b Compare May 5, 2023 11:15
@metachris
Copy link
Collaborator Author

Deployed to Flashbots Goerli and Sepolia relays

jtraglia
jtraglia previously approved these changes May 5, 2023
Copy link
Collaborator

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few little nits, but LGTM 👍 I'm all for performance improvements.

@@ -1387,7 +1411,7 @@ func (api *RelayAPI) handleSubmitNewBlock(w http.ResponseWriter, req *http.Reque
log.Warn("could not find slot duty")
api.RespondError(w, http.StatusBadRequest, "could not find slot duty")
return
} else if slotDuty.Entry.Message.FeeRecipient.String() != payload.ProposerFeeRecipient() {
} else if !strings.EqualFold(slotDuty.Entry.Message.FeeRecipient.String(), payload.ProposerFeeRecipient()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now a case-insensitive equality comparison. Makes sense, because the SSZ payload is just bytes 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, was needed to work with both JSON and SSZ :)

common/ssz_test.go Outdated Show resolved Hide resolved
common/ssz_test.go Outdated Show resolved Hide resolved
services/api/service_test.go Outdated Show resolved Hide resolved
jtraglia
jtraglia previously approved these changes May 5, 2023
Copy link
Collaborator

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks 😄

Copy link
Collaborator

@michaelneuder michaelneuder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ssz is a great!

req.Header.Set(k, v)
}

// lfg
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: lfg?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's fucking go 🤠

@codecov-commenter
Copy link

codecov-commenter commented May 7, 2023

Codecov Report

Merging #388 (2d51c47) into main (75f6c16) will increase coverage by 2.10%.
The diff coverage is 53.57%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #388      +/-   ##
==========================================
+ Coverage   20.35%   22.46%   +2.10%     
==========================================
  Files          21       21              
  Lines        4082     4104      +22     
==========================================
+ Hits          831      922      +91     
+ Misses       3149     3055      -94     
- Partials      102      127      +25     
Flag Coverage Δ
unittests 22.46% <53.57%> (+2.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
services/api/service.go 22.58% <53.57%> (+5.71%) ⬆️

... and 1 file with indirect coverage changes

@metachris metachris merged commit 343c03b into main May 8, 2023
2 checks passed
@metachris metachris deleted the ssz-builder-submission branch May 8, 2023 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: allow block builder submissions with ssz payload, and with json+gzip
4 participants