op-node: Implement fjord RLP & Channel Bank Size limit Increases #10357
Conversation
This also creates a ChainSpec object which is responsible for returning protocol parameters. We use a different object than the rollup.Config because the config is primarily a disk representation & does not concern itself with protocol constants.
trianglesphere
force-pushed
the
jg/impl_fjord_increases
branch
from
a919df5 to
aa5a062
Compare
|
Semgrep found 1 Service 'op_stack_go_builder' is running with a writable root filesystem. This may allow malicious applications to download and run additional payloads, or modify container files. If an application inside a container has to save something temporarily consider using a tmpfs. Add 'read_only: true' to this service to prevent this. Ignore this finding from writable-filesystem-service.Semgrep found 1 Service 'op_stack_go_builder' allows for privilege escalation via setuid or setgid binaries. Add 'no-new-privileges:true' in 'security_opt' to prevent this. Ignore this finding from no-new-privileges. |
trianglesphere
requested review from
protolambda,
ajsutton and
a team
as code owners
WalkthroughWalkthroughThe changes across multiple files involve transitioning from Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (17)
Files skipped from review as they are similar to previous changes (16)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (1)
op-node/rollup/derive/channel_in_reader.go (1)
20-25: Ensure the transition fromConfigtoChainSpecinChannelInReaderdoes not affect functionality.The structural change to use
ChainSpecshould be carefully tested to ensure that it integrates well with existing functionalities and does not introduce regressions.
trianglesphere
force-pushed
the
jg/impl_fjord_increases
branch
from
aa5a062 to
8cb879b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (2)
op-node/rollup/derive/span_batch_util.go (2)
Line range hint
9-9: Update the function comment to reflect the actual behavior or constraints.The comment mentions
MaxSpanBatchSize, which is not used in the function. Consider updating the comment to avoid confusion.
Line range hint
25-25: Update the function comment to reflect the actual behavior or constraints.The comment mentions
MaxSpanBatchSize, which is not used in the function. Consider updating the comment to avoid confusion.
trianglesphere
force-pushed
the
jg/impl_fjord_increases
branch
from
8cb879b to
a8e5714
Compare
There was a problem hiding this comment.
LGTM, I like the introduction of a spec.
I have a few open questions that might lead to changes.
If we consume only an interface of the spec (or even only the value getters from the spec that we need), it might become easier in tests to set low values, as the spec can then be mocked.
Description
Implements ethereum-optimism/specs#165 & ethereum-optimism/specs#170 in the op-node.
I made several changes to span batch parsing which I believe to be spec compliant.
MAX_SPAN_BATCH_ELEMENT_COUNT). That means that the largest buffer possible to allocate is 1.25 million bytes while the code was checking against a size limit of 10 million bytes.Tests
I added chain spec tests. No tests inside the channel bank / channel reader; however I made the old constant private so they can only be accessed through these function.
Metadata