feat: add support for alt-da with da-server #69
Conversation
README.md
Outdated
| # Available services: | ||
| # - blockscout | ||
| # - da_server | ||
| additional_services: [] | ||
|
|
||
| # Configuration place for da-server - https://github.com/ethereum-optimism/optimism/tree/develop/op-alt-da | ||
| da_server_params: | ||
| # A list of optional extra params that will be passed to the da-server container for modifying its behaviour | ||
| da_server_extra_args: [] | ||
| generic_commitment: false |
There was a problem hiding this comment.
Right now adding da_server to the additional_services list will by default turn on alt-da with default keccakCommitment mode. the commitment mode can be changed to generic_commitment in the da_server_params.
Given that alt-da and commitment mode and network parameters that affect other services (op-node at least), I feel like they should be somewhere more prominent. Wasn't sure if the network_params section of the config was the right place though. Also doing this might require double checking the settings passed to da_server_params to make sure they agree.
| @@ -162,6 +165,9 @@ def get_beacon_config( | |||
| "--p2p.listen.ip=0.0.0.0", | |||
| "--p2p.listen.tcp={0}".format(BEACON_DISCOVERY_PORT_NUM), | |||
| "--p2p.listen.udp={0}".format(BEACON_DISCOVERY_PORT_NUM), | |||
| "--altda.enabled=" + str(da_server_context.enabled), | |||
There was a problem hiding this comment.
Similar check would be needed here instead of adding it by default to all nodes irrespective of if the da server is present
There was a problem hiding this comment.
see my other comment. enabled is set to false by default, so its safe to add by default (and is what the other docker-compose devnet was doing).
| da_server_extra_args, | ||
| generic_commitment, | ||
| ): | ||
| image = "us-docker.pkg.dev/oplabs-tools-artifacts/images/da-server:devnet" |
There was a problem hiding this comment.
the devnet tag doesn't exist and is overriding the DEFAULT in the input parser. In general I'd avoid overriding it this way. I'd also add the da_image as a config variable under da_server_params and then set the image return value for ServiceConfig as image=da_server_params.da_server_image. The input parser will populate the default value if none is set, so by default the latest image is used and if specified the config image is used.
| "--addr=0.0.0.0", | ||
| "--port=3100", | ||
| "--log.level=debug", | ||
| "--generic-commitment=" + str(generic_commitment), |
There was a problem hiding this comment.
da_server image complains on this at startup: Incorrect Usage: flag provided but not defined: -generic-commitment
I'd image the latest image found here us-docker.pkg.dev/oplabs-tools-artifacts/images/da-server:latest is stale and missing the flag?
There was a problem hiding this comment.
Yes confirmed that the latest image is missing the flag. And the more recent one v0.1.0-rc.1 from June gives me this error when I try to run on my mac: rosetta error: failed to open elf at /lib/ld-musl-x86_64.so.1
There was a problem hiding this comment.
WIth the latest kurtosis 1.3 release I was able to add optimism repo as a submodule and build the da-server image locally: e709d7c
|
A modern |
fix: type bugs in da_server related configs chore: refactor da-server launcher to not hardcode image feat: add optimism repo as submodule to build da-server image revert: changes to network_params.yaml
|
@mslipper can we sync on what's needed to make this work? Have some questions after the rebase about:
Currently running into Also not sure why 5-8 enclaves get created now. Seems like most of them are empty anyways? |
For simplicity we should enforce all altda or all rollup. Adding a single altda chain to a cluster would essentially make all of them have altda levels of security |
Closes #67
Mimicing the behavior of the docker-compose devnet.
Testing
Only manually deployed everything, and checked that the op-batcher and da-server logs were normal.