-
Notifications
You must be signed in to change notification settings - Fork 5
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
Increase MaxInputSize to 16MB in op-alt-da #264
base: celo10
Are you sure you want to change the base?
Conversation
On `celo10` branch I've observed in the batcher: ``` {"t":"2024-10-17T15:24:33.473665645Z","lvl":"crit","msg":"Application failed","message":"failed to setup: failed to init channel config: max frame size 999999 exceeds altDA max input size 130672"} ``` With previous `celo` versions this size limit was not enforced, and we increased the max frame size to 16MB in the derivation pipeline ([PR](#228)).
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
@jcortejoso Do we still want this? |
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
@gastonponti Please have a look. It say's this is a hardfork, so we should do this now if we want to have this change. But what happens if we need to fallback to CallData DA and this is increased? |
The fallback to ethDa depends on the Frame size, which is not this config as far as I understand. optimism/op-batcher/batcher/config.go Line 57 in 0df9d40
This one is the one we MUST NOT change if we plan to have the fallback. This seems to be the maximum blob size, which could have multiple Frames. The problem I'm seeing, is that the PR (#228) that @jcortejoso pointed, is moving that max frame to 16M (which was 1M but saying in the comments that it shouldn't be more than 128kb but it was set to 1M to future changes), which makes me think that we are changing the Frame size as part of our configuration, which it could be a problem. So, this doesn't seem to be a problem for the fallback (don't know about the fault proofs), but if we are changing the Frame size, the fallback won't work |
On
celo10
branch I've observed in the batcher:With previous
celo
versions this size limit was not enforced, and we increased the max frame size to 16MB in the derivation pipeline (PR).