-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix: change put route put/ -> put #168
Conversation
cc @karlb for review. Just applied the patch you had shared with me here. |
Not sure why the test is still failing. @karlb said he would take a look tomorrow. |
// we need to handle both: see https://github.com/ethereum-optimism/optimism/pull/12081 | ||
// /put is for generic commitments, and /put/ for keccak256 commitments | ||
// TODO: we should probably separate their handlers? | ||
mux.HandleFunc("/put", WithLogging(WithMetrics(svr.HandlePut, svr.m), svr.log)) | ||
mux.HandleFunc("/put/", WithLogging(WithMetrics(svr.HandlePut, svr.m), svr.log)) |
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.
@epociask can you review that this comment and the routes make sense. tests are passing but still want to be 100% certain since I still don't get our routing logic with the different paths. Should probably document this api at some point..
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.
lgtm
|
||
require ( | ||
github.com/Layr-Labs/eigenda v0.8.4 | ||
github.com/consensys/gnark-crypto v0.12.1 | ||
github.com/ethereum-optimism/optimism v1.9.2 | ||
github.com/ethereum/go-ethereum v1.14.8 | ||
github.com/ethereum-optimism/optimism v1.9.4-0.20240927020138-a9c7f349d10b |
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.
@samlaf Remember to change to a canonical tag after 1.9.4 is released by OP
Fixes Issue
Fixes #88
Changes proposed
Screenshots (Optional)
Note to reviewers
Updated to optimism 1.9.3 which required also updating to go1.22