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

feat: Eigenda Config #307

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Conversation

gianbelinche
Copy link

What ❔

This PR adds some new configs necessary now that the proxy is not going to be used anymore.
We may need more or less fields in the future, this is just to get ahead.
Some configs like api_node_url will need to be removed once we remove the eigenda proxy, but for now to preserve functionality until rust proxy is finished I just left comments were removal would be necessary.
There are two variants:

da_client:
  eigen_da:
    memstore:
      api_node_url: http://127.0.0.1:4242 # TODO: This should be removed
      max_blob_size_bytes: 2097152
      blob_expiration: 100000
      get_latency: 100
      put_latency: 100
da_client:
  eigen_da:
    disperser:
      api_node_url: http://127.0.0.1:4242 # TODO: This should be removed
      disperser_rpc: <your_desired_disperser>
      eth_confirmation_depth: -1
      eigenda_eth_rpc: <your_desired_rpc>
      eigenda_svc_manager_addr: '0xD4A7E1Bd8015057293f0D0A557088c286942e84b'

Why ❔

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk_supervisor fmt and zk_supervisor lint.

Copy link

@juan518munoz juan518munoz left a comment

Choose a reason for hiding this comment

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

Looks Ok, but would take comment into account, an incorrect parsing in the config (due to a typo for example) might be a headache later on if we don't catch it as soon as possible.

DAClientConfig::EigenDA(config) => {
self.node.add_layer(EigenDAProxyLayer::new(config));
}
_ => {}

Choose a reason for hiding this comment

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

loading a config, but not matching to any da_config should result in an Error.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think it should be an error, if you are using avail for example, you don't want the proxy layer to be added, this takes care of that.
You cannot add it, since it would later need the specific config for eigenda, which it would not have

@juanbono juanbono merged commit 4a19f54 into da-eigen-implementation-m0 Oct 18, 2024
11 of 23 checks passed
@juanbono juanbono deleted the eigenda-proxy-config branch October 18, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants