-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
op-batcher: Support generic DA posting #10489
op-batcher: Support generic DA posting #10489
Conversation
…respect op-plasma spec
… to specify da svc
…nt propagation from test da server
…op-plasma/generic-da-commitment
…ssertion when reading in op-node
…c DA client <-> interactions
…c DA client <-> interactions
WalkthroughWalkthroughThe changes involve significant updates to the handling of Data Availability (DA) services across multiple components. Key updates include the addition of a new flag for DA service type in the CLI, modifications to the Changes
Assessment against linked issues
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (2)
Files skipped from review as they are similar to previous changes (1)
Additional Context UsedRuff (6)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
…op-plasma/generic-da-commitment
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.
Actionable comments posted: 0
Out of diff range and nitpick comments (5)
bedrock-devnet/devnet/__init__.py (5)
Line range hint
7-7
: Remove unused importcalendar
.-import calendar
Line range hint
12-12
: Remove unused importgzip
.-import gzip
Line range hint
18-18
: Remove unused importdevnet.log_setup
.-import devnet.log_setup
Line range hint
221-221
: Avoid equality comparisons toFalse
; useif not os.path.exists(l2_allocs_path):
for false checks.- if os.path.exists(l2_allocs_path) == False or DEVNET_FPAC == True: + if not os.path.exists(l2_allocs_path) or DEVNET_FPAC:
Line range hint
312-312
: Local variablee
is assigned to but never used. Consider removing it or using it in the logging statement.- except Exception as e: + except Exception:
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.
TY for implementing this
db82b31
* op-batcher: Support generic DA commitment type * op-batcher: Support generic DA commitment type - update generic flag info * op-batcher: Support generic DA commitment type - change flag name to respect op-plasma spec * op-batcher: Support generic DA commitment type - update devnet script to specify da svc * op-batcher: Support generic DA commitment type - fix failing multi challenge test * op-batcher: Support generic DA commitment type - fix failing multi challenge test * op-batcher: Support generic DA commitment type - rm dbug regression in devnet script * op-batcher: Support generic DA commitment type - fix bugs in commitment propagation from test da server * op-batcher: Support generic DA commitment type - rm commitment type assertion when reading in op-node * op-batcher: Support generic DA commitment type - rm dead code * op-batcher: Support generic DA commitment type - disable da service in devnet script * op-batcher: Support generic DA commitment type - less verbose comments * op-batcher: Support generic DA commitment type - add tests for generic DA client <-> interactions * op-batcher: Support generic DA commitment type - add tests for generic DA client <-> interactions * op-batcher: Support generic DA commitment type - title case cli var
Description
op-batcher
DA_SERVICE
flag that specifies whether to use precomputed commitments or an alt DA service providerop-node
Keccak256
typeop-plasma
daClient
to support writing without a precomputed commitmentdaServer
to support both interaction types based on prevalence of commitment query param in URL pathdaServer
with more verbose logging to ease debugging / devnet introspectiondevnet
docker-compose.yml
to useDA_SERVICE
flag as perop-node
andop-batcher
env vars__init__.py
to setDA_SERVICE
flag in compose environmentExtend
Tests
DA_SERVICE=true
in compose environment, tests were ran as per commit (6a1ac766a1ac76) and passed for the alternative DA service interaction. See test run hereda_client.go
tests to assert put/get interactions using generic commitment typeAdditional context
Metadata