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: add sequencer dangerous flag to disable tx size checks #2885

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kevin-satsuma
Copy link

adding the disable-seq-inbox-max-data-size-check flag under a new dangerous field in the sequencer config. this config disables the check in nitro.go that compares the sequencer MaxTxDataSize to ensure that it is at least 15kB below the sequencer inbox's MaxDataSize.

discussions with offchain labs on slack helped clarify that this should be a dangerous flag because it breaks batcher manual fallbacks in case the DA layer is down. if the sequencer MaxTxDataSize is too high, there is no way the txs(and further the batches) into the sequencer inbox on the parent chain in a manual fallback scenario.

had some difficulties with the naming of this flag to fit all of the needed context. the sequencer inbox MaxTxDataSize is checked two times. once against the sequencer inbox MaxDataSize(in nitro.go, we are trying to disable this one) and the other time against arbostypes.MaxL2MessageSize(in sequencer.go, not disabling this check). it can be quite verbose to try to spell out the entire interaction(like DisableMaxTxDataSizeCheckAgainstSeqInboxMaxDataSize) to avoid confusion on the exact check that is being targeted. on the other hand, i dont see any precedent in any other configs to nest the dangerous section(like sequencer.dangerous.max-tx-data-size.disable-seq-inbox-max-data-size-check). for now, i shortened it down to DisableSeqInboxMaxDataSizeCheck, but i understand this can leave some ambiguity. as this is my first contribution to this repo, please let me know what kind of style you all prefer.

with this new dangerous flag, the new limit for sequencer MaxTxDataSize should be ~212kB(arbostypes.MaxL2MessageSize-50000).

internal context for the offchain labs team: https://alchemyinsights.slack.com/archives/C06SZ7EKS2H/p1736987075713719

adding the `disable-seq-inbox-max-data-size-check` flag under a new `dangerous` field in the sequencer config. this config disables the check in `nitro.go` that compares the sequencer MaxTxDataSize to ensure that it is at least 15kB below the sequencer inbox's MaxDataSize.

discussions with offchain labs on slack helped clarify that this should be a dangerous flag because it breaks batcher manual fallbacks in case the DA layer is down. if the sequencer MaxTxDataSize is too high, there is no way the txs(and further the batches) into the sequencer inbox on the parent chain in a manual fallback scenario.

had some difficulties with the naming of this flag to fit all of the needed context. the sequencer inbox MaxTxDataSize is checked two times.  once against the sequencer inbox MaxDataSize(in `nitro.go`, we are trying to disable this one) and the other time against arbostypes.MaxL2MessageSize(in `sequencer.go`, not disabling this check). it can be quite verbose to try to spell out the entire interaction(like `DisableMaxTxDataSizeCheckAgainstSeqInboxMaxDataSize`) to avoid confusion on the exact check that is being targeted. on the other hand, i dont see any precedent in any other configs to nest the `dangerous` section(like `sequencer.dangerous.max-tx-data-size.disable-seq-inbox-max-data-size-check`). for now, i shortened it down to `DisableSeqInboxMaxDataSizeCheck`, but i understand this can leave some ambiguity.  as this is my first contribution to this repo, please let me know what kind of style you all prefer.

with this new dangerous flag, the new limit for sequencer MaxTxDataSize should be ~212kB(`arbostypes.MaxL2MessageSize-50000`).

internal context for the offchain labs team: https://alchemyinsights.slack.com/archives/C06SZ7EKS2H/p1736987075713719
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants