Integrate Espresso proxy#90
Conversation
|
@codex review |
| ESPRESSO_QUERY_SERVICE_URL= | ||
| ESPRESSO_LIGHT_CLIENT_ADDRESS=0x303872bb82a191771321d4828888920100d0b3e4 | ||
| BATCHER_ADDR= | ||
| BATCH_AUTH_ADDR= | ||
| ESPRESSO_INITIAL_HOTSHOT_HEIGHT= | ||
| ESPRESSO_TAG= |
There was a problem hiding this comment.
The approach we used previously for the network specific env files was to include here things that were network specific and would not change. The remaining config options which operators might actually want to configure themselves we would put in the network env file in the root of the repo.
ESPRESSO_TAG seems like something that operators would want to configure and I'm also wondering about the ESPRESSO_QUERY_SERVICE_URL, is that something that would be fixed per network or something that operators may want to customise?
There was a problem hiding this comment.
Yes this is true, I will move the ESPRESSO_TAG to network specific config and ESPRESSO_QUERY_SERVICE_URL is something which can be fixed so will add those fixed values here 🙌
| ESPRESSO_QUERY_SERVICE_URL= | ||
| ESPRESSO_LIGHT_CLIENT_ADDRESS=0x95Ca91Cea73239b15E5D2e5A74d02d6b5E0ae458 | ||
| BATCHER_ADDR= | ||
| BATCH_AUTH_ADDR= | ||
| ESPRESSO_INITIAL_HOTSHOT_HEIGHT= | ||
| ESPRESSO_TAG= |
There was a problem hiding this comment.
This repo is intended to provide a zero configuration approach to running a celo node, simply copy the env for the given network and run docker compose up, so these empty vars are breaking that, but it seems like all of them should be able to be set with sensible defaults.
There was a problem hiding this comment.
Yes makes sense! As discussed we can fill out some of these values as we are close to the migration which will help the proxy sync faster
| ``` | ||
|
|
||
|
|
||
| ## Espresso Rollup Node Proxy |
There was a problem hiding this comment.
This section is out of place in that it's wedged in between two ### headings, under advanced configuration.
My suggestion would be to Introduce a flag that enables espresso (ESPRESSO_ENABLED), similar to the MONITORING_ENABLED flag and then add a line under Key Environment Variables with a short description of the effect of ESPRESSO_ENABLED and link to a sub heading coming after ### P2P Networking Environment Variables that describes espresso functionality and config.
| To use a specific version of the Espresso proxy image: | ||
|
|
||
| ```text | ||
| IMAGE_TAG__ESPRESSO_PROXY=<tag> | ||
| ``` | ||
|
|
||
| If unset, the default tag (`v0.0.1`) is used. |
There was a problem hiding this comment.
This seems unnecessary, since the mechanism is already described above (line 109). It's also liable to get stale since this would need to be updated in tandem with the version defined in the docker compose file. So I'd be in favour of removing this section.
| @@ -0,0 +1,6 @@ | |||
| ESPRESSO_QUERY_SERVICE_URL= | |||
| ESPRESSO_LIGHT_CLIENT_ADDRESS=0x95Ca91Cea73239b15E5D2e5A74d02d6b5E0ae458 | |||
| BATCHER_ADDR= | |||
There was a problem hiding this comment.
I'm wondering why a BATCHER_ADDRESS is required here, given that espresso batches are authenticated based on their signature and not on the address of the batcher?
There was a problem hiding this comment.
The proxy checks that the messages that are finalized by Espresso are from an authorized TEE batcher address thats why the batcher addr is needed here ( its the tee batcher addr) I will improve naming here
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31bd8342d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Note: Any request that does not contain the configured Espresso tag is proxied to op-geth without interception, so all standard JSON-RPC functionality is preserved. | ||
|
|
||
| The proxy waits for op-geth to fully sync before starting and stores its state in a persistent volume. |
There was a problem hiding this comment.
I'm wondering how long syncing will take, potentially it makes sense to allow the proxy to sync while op-geth is syncing?
There was a problem hiding this comment.
Proxy wont be able to reliably sync while the op geth is syncing because for proxy to actually advance the espresso finalized block, it needs to first get that block from op-geth and then check if it matches with what Espresso finalized.
| @@ -0,0 +1,5 @@ | |||
| ESPRESSO_QUERY_SERVICE_URL=https://query.decaf.testnet.espresso.network/ | |||
| ESPRESSO_LIGHT_CLIENT_ADDRESS=0x303872bb82a191771321d4828888920100d0b3e4 | |||
| TEE_BATCHER_ADDR= | |||
There was a problem hiding this comment.
will fill these values later once we have them available
| ESPRESSO_LIGHT_CLIENT_ADDRESS=0x303872bb82a191771321d4828888920100d0b3e4 | ||
| TEE_BATCHER_ADDR= | ||
| BATCH_AUTH_ADDR= | ||
| ESPRESSO_INITIAL_HOTSHOT_HEIGHT= |
There was a problem hiding this comment.
I'm wondering if this is needed to be set explicitly, it seems like it could be automatically set by calling the espresso light client on the L1 to fetch a recently finalized espresso block?
3 participants
Description
This PR integrates Espresso proxy with celo docker compose setup
Changes