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

Spec and implementation disagree about URL #11499

Closed
karlb opened this issue Aug 16, 2024 · 1 comment · Fixed by #12081
Closed

Spec and implementation disagree about URL #11499

karlb opened this issue Aug 16, 2024 · 1 comment · Fixed by #12081

Comments

@karlb
Copy link
Contributor

karlb commented Aug 16, 2024

The OP alt-DA spec requires

Request:
  POST /put
  Content-Type: application/octet-stream
  Body: <preimage_bytes>

Response:
  200 OK
  Content-Type: application/octet-stream
  Body: <hex_encoded_commitment>

But the implementation only listens at the URL /put/ (with trailing slash). Should the implementation change or the spec?

@tynes
Copy link
Contributor

tynes commented Aug 20, 2024

This is a good catch. I lean towards saying the spec is correct and updating the implementation to handle either /put or /put/

karlb added a commit to celo-org/optimism that referenced this issue Sep 24, 2024
In addition to the currently used `/put/` path to ease the migration.
See ethereum-optimism#11499.
karlb added a commit to celo-org/optimism that referenced this issue Sep 24, 2024
The spec mandates using `/put` and not `/put/`, so that is what we
should do.

Warning: This will break DA solutions that only accept `/put/` at the
moment. It is recommended that DA solutions support both paths for a
while, so that updating OP-stack can happen independently of the exact
DA implementation version.

Closes ethereum-optimism#11499.
karlb added a commit to celo-org/optimism that referenced this issue Sep 24, 2024
In addition to the currently used `/put/` path to ease the migration.
See ethereum-optimism#11499.
karlb added a commit to celo-org/optimism that referenced this issue Sep 24, 2024
The spec mandates using `/put` and not `/put/`, so that is what we
should do.

Warning: This will break DA solutions that only accept `/put/` at the
moment. It is recommended that DA solutions support both paths for a
while, so that updating OP-stack can happen independently of the exact
DA implementation version.

Closes ethereum-optimism#11499.
github-merge-queue bot pushed a commit that referenced this issue Sep 27, 2024
* Accept /put path as described in spec

In addition to the currently used `/put/` path to ease the migration.
See #11499.

* alt-DA: write to /put path as described in spec

The spec mandates using `/put` and not `/put/`, so that is what we
should do.

Warning: This will break DA solutions that only accept `/put/` at the
moment. It is recommended that DA solutions support both paths for a
while, so that updating OP-stack can happen independently of the exact
DA implementation version.

Closes #11499.
samlaf pushed a commit to samlaf/optimism that referenced this issue Nov 10, 2024
* Accept /put path as described in spec

In addition to the currently used `/put/` path to ease the migration.
See ethereum-optimism#11499.

* alt-DA: write to /put path as described in spec

The spec mandates using `/put` and not `/put/`, so that is what we
should do.

Warning: This will break DA solutions that only accept `/put/` at the
moment. It is recommended that DA solutions support both paths for a
while, so that updating OP-stack can happen independently of the exact
DA implementation version.

Closes ethereum-optimism#11499.
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 a pull request may close this issue.

2 participants