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

go/batch-submitter: HTTP/2 fixes #2078

Merged
merged 2 commits into from Jan 31, 2022

Conversation

mslipper
Copy link
Collaborator

The built-in geth client has a bugger implementation of HTTP/2: ethereum/go-ethereum#24292. Additionally, Go 1.17 has better HTTP/2 support than Go 1.15. This PR updates the BSS to use Go 1.17 in built binaries, and adds a flag to disable HTTP/2 support if necessary.

The built-in geth client has a bugger implementation of HTTP/2: ethereum/go-ethereum#24292. Additionally, Go 1.17 has better HTTP/2 support than Go 1.15. This PR updates the BSS to use Go 1.17 in built binaries, and adds a flag to disable HTTP/2 support if necessary.
@changeset-bot
Copy link

changeset-bot bot commented Jan 25, 2022

🦋 Changeset detected

Latest commit: f0c3f93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/batch-submitter-service Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Copy link
Contributor

@tynes tynes left a comment

Choose a reason for hiding this comment

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

Ideally we won't need to run with disabling http2 once the next version of geth is released with that bug fix. Perhaps scope creep, but i don't think its a bad idea to configure the http client with a user agent that includes the version, this could be done simply by changing disableHTTP2 into httpOpts and calling this: https://github.com/ethereum/go-ethereum/blob/015fde9a2c4b6e722a9de5bfea2a14226806a231/rpc/client.go#L267

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2022

Codecov Report

Merging #2078 (f0c3f93) into develop (dc1ed3c) will decrease coverage by 1.97%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2078      +/-   ##
===========================================
- Coverage    77.35%   75.37%   -1.98%     
===========================================
  Files           57       81      +24     
  Lines         1983     2705     +722     
  Branches       292      436     +144     
===========================================
+ Hits          1534     2039     +505     
- Misses         449      666     +217     
Flag Coverage Δ
batch-submitter 62.63% <ø> (ø)
contracts 90.48% <ø> (ø)
core-utils 59.94% <ø> (ø)
data-transport-layer 38.64% <ø> (?)
message-relayer 70.86% <ø> (?)
sdk 86.34% <ø> (?)

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

Impacted Files Coverage Δ
packages/sdk/src/utils/misc-utils.ts 100.00% <0.00%> (ø)
...kages/data-transport-layer/src/utils/validation.ts 10.71% <0.00%> (ø)
...ices/l1-ingestion/handlers/transaction-enqueued.ts 53.84% <0.00%> (ø)
packages/data-transport-layer/src/utils/index.ts 100.00% <0.00%> (ø)
packages/sdk/src/utils/contracts.ts 93.33% <0.00%> (ø)
.../src/services/l2-ingestion/handlers/transaction.ts 60.86% <0.00%> (ø)
packages/sdk/src/utils/coercion.ts 88.46% <0.00%> (ø)
packages/sdk/src/utils/index.ts 100.00% <0.00%> (ø)
packages/sdk/src/cross-chain-messenger.ts 66.12% <0.00%> (ø)
packages/sdk/src/cross-chain-provider.ts 87.67% <0.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc1ed3c...f0c3f93. Read the comment docs.

@mslipper mslipper merged commit a6afe97 into ethereum-optimism:develop Jan 31, 2022
tynes added a commit that referenced this pull request Feb 25, 2022
Update to go-ethereum v1.10.16.
Release notes:
https://github.com/ethereum/go-ethereum/releases/tag/v1.10.16

This notably fixes the http2 bug in the rpc client.

#2078 will
be able to be reverted after this is merged in
tynes added a commit that referenced this pull request Feb 25, 2022
Update to go-ethereum v1.10.16.
Release notes:
https://github.com/ethereum/go-ethereum/releases/tag/v1.10.16

This notably fixes the http2 bug in the rpc client.

#2078 will
be able to be reverted after this is merged in
nebojsa94 pushed a commit to Tenderly/optimism that referenced this pull request Apr 26, 2022
Update to go-ethereum v1.10.16.
Release notes:
https://github.com/ethereum/go-ethereum/releases/tag/v1.10.16

This notably fixes the http2 bug in the rpc client.

ethereum-optimism#2078 will
be able to be reverted after this is merged in
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ops Area: ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants