docs: enhance Execution API documentation with block size management #2215
Conversation
WalkthroughThis change revises the ADR (Architecture Decision Record) document for the Rollkit Execution API. The update elaborates on the API’s design, rationale, and operational expectations, including expanded sections on context, alternative approaches, API behaviors, state management, and block size management. New implementation guidelines and references are added, while previous notes on Rollkit’s internal code changes and testing are removed. The consequences section is updated with additional points regarding flexibility, complexity, and coordination. No code or exported entity signatures are modified; the changes are limited to documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Rollkit
participant ExecutionAPI
participant Sequencer
participant DA_Layer
Client->>Rollkit: Submit Transaction
Rollkit->>ExecutionAPI: ExecuteTxs (with txs)
ExecutionAPI-->>Rollkit: Execution result, maxBytes
Rollkit->>Sequencer: Propose Block (respecting maxBytes)
Sequencer->>DA_Layer: Submit Block
DA_Layer-->>Sequencer: Confirm Block
Sequencer-->>Rollkit: Block Finalized
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
tzdybal
force-pushed
the
tzdybal/adr-010-improvements
branch
from
855bb83 to
bb04675
Compare
|
tzdybal
force-pushed
the
tzdybal/adr-010-improvements
branch
from
bb04675 to
f4772f7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2215 +/- ##
==========================================
- Coverage 44.96% 44.92% -0.05%
==========================================
Files 48 48
Lines 4561 4561
==========================================
- Hits 2051 2049 -2
- Misses 2290 2292 +2
Partials 220 220 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
RollkitBot
requested review from
a team,
Manav-Aggarwal,
gupadhyaya and
tuxcanfly
and removed request for
a team
|
The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
specs/lazy-adr/adr-010-exec-api.md (4)
25-26: Suggest Article in ABCI Application Reference
The phrase “implement other VMs inside ABCI application” could read more naturally as “inside an ABCI application” or “inside the ABCI application.”
111-111: Clarify “Properly Formatted” Requirement
Please specify what “properly formatted” entails (e.g., field order, encoding, schema version) or reference a schema/validator to avoid ambiguity.
161-162: Reevaluate Storage Optimization Placement
The note on “Optimize storage for finalized blocks” may be better placed under performance guidelines to keep behavioral specs focused on immutability guarantees.
193-197: Detail Protocol Overhead Buffer Calculation
Consider specifying how the overhead buffer is computed (fixed byte count vs. percentage) or providing a formula to avoid implementation discrepancies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
specs/lazy-adr/adr-010-exec-api.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
specs/lazy-adr/adr-010-exec-api.md
[uncategorized] ~24-~24: You might be missing the article “the” here.
Context: ...nterface and implement other VMs inside ABCI application. - Pros: No changes requ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~200-~200: This phrase is duplicated. You should probably use “the sequencer” only once.
Context: ...alue from the DA layer and passes it to the sequencer - The sequencer uses this value to limit the size of tr...
(PHRASE_REPETITION)
[duplication] ~214-~214: Possible typo: you repeated a word.
Context: ...on environments must maintain their own state - State transitions should be atomic - State...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~310-~310: You might be missing the article “a” here.
Context: ...yer. ### Neutral 1. Need to introduce new API exposed by rollkit. 2. Changes to e...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: rollkit-docker / docker-build (GHCR; ghcr.io/rollkit/rollkit)
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (20)
specs/lazy-adr/adr-010-exec-api.md (20)
6-6: Changelog Entry Addition
The new changelog entry for “2025.04.24: Various improvements” accurately records the PR’s date and scope of changes.
13-21: Enhance Context with Execution API Bridge Explanation
The added paragraph clearly describes the Execution API as a bridge between Rollkit and diverse VMs, and the bulleted list concisely enumerates its benefits.
28-29: Approach 2 Pros and Cons are Clear
The pros and cons for migrating to the Engine API are well-balanced and succinct.
31-32: Approach 3 Pros and Cons are Well-Defined
The analysis for creating a new generic API clearly outlines the trade-offs.
88-88: InitChain Behavior Clarified
Adding “Ensure all necessary state is initialized for subsequent operations” strengthens the specification by explicitly requiring post-genesis readiness.
140-140: Atomic Execution Guarantee Added
Mandating atomic execution (“either all transactions succeed or none do”) greatly improves correctness guarantees.
169-170: General Notes on State Management and Atomicity
The general notes now correctly emphasize the execution environment’s state responsibility and atomicity of state transitions.
181-181: Add Block Size Management Section
Introducing this new section draws necessary attention to howmaxBytesis handled across the API.
183-184: IntroducemaxBytesParameter Role
DefiningmaxBytesas a return value in both methods immediately contextualizes its importance in block size enforcement.
185-192: Clarify Dynamic Adjustment Mechanism
How should Rollkit handle decreases inmaxBytesbetween blocks (e.g., trimming pending batches or applying backpressure)? Please verify that edge cases are documented.
199-205: Clarify Source of InitialmaxBytes
Earlier you state thatInitChainreturnsmaxBytesfrom the execution environment, but here it reads as coming from the DA layer. Please confirm the authoritative source and update for consistency.🧰 Tools
🪛 LanguageTool
[grammar] ~200-~200: This phrase is duplicated. You should probably use “the sequencer” only once.
Context: ...alue from the DA layer and passes it to the sequencer - The sequencer uses this value to limit the size of tr...(PHRASE_REPETITION)
207-210: Rollkit Usage ofmaxBytesIs Clear
The guidance on how Rollkit validates and applies block size limits before submission is concise and actionable.
213-218: Implementation Guidelines—State Management
These bullets cover persistence, crash recovery, and atomic transitions thoroughly.🧰 Tools
🪛 LanguageTool
[duplication] ~214-~214: Possible typo: you repeated a word.
Context: ...on environments must maintain their own state - State transitions should be atomic - State...(ENGLISH_WORD_REPEAT_RULE)
219-224: Implementation Guidelines—Error Handling
Error handling best practices are clearly outlined, including context cancellation and cleanup on failures.
225-230: Implementation Guidelines—Performance Considerations
Performance tips like caching and minimizing state transitions are appropriate for high-throughput environments.
231-236: Implementation Guidelines—Security
Security requirements (validation, access control, sensitive data handling) are comprehensive and align with best practices.
298-300: Positive Consequences Updated
The new points on flexibility and clearer boundaries accurately describe the architectural benefits.
305-307: Negative Consequences Expanded
Including coordination complexity and abstraction overhead presents a balanced view of potential drawbacks.
311-312: Neutral Consequences Added
The neutral observations around API introduction and tooling updates are well noted.
317-317: References Section Enhanced
Linking directly to thego-executionrepository gives readers immediate access to the API definitions.
Manav-Aggarwal
left a comment
Manav-Aggarwal
left a comment
There was a problem hiding this comment.
Thanks! Left some comments.
…details Added a section on block size management to the Execution API documentation, detailing the role of the `maxBytes` parameter in `InitChain` and `ExecuteTxs` methods. This update clarifies how the parameter aids in maintaining consistent block size constraints and its usage in Rollkit for validating block sizes before submission.
tzdybal
force-pushed
the
tzdybal/adr-010-improvements
branch
from
2325ceb to
88586a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
specs/lazy-adr/adr-010-exec-api.md (4)
166-167: Avoid duplication of state-management guidance
The “State Management” and “Atomicity” bullets here duplicate points in the later Implementation Guidelines. Consider consolidating to a single section to reduce redundancy.
178-206: Well-structured Block Size Management section
This comprehensive overview ofmaxBytesusage across InitChain and ExecuteTxs clarifies block-size enforcement. Consider adding a note on the unit (bytes) and explicit handling of protocol overhead calculations for even greater clarity.🧰 Tools
🪛 LanguageTool
[grammar] ~197-~197: This phrase is duplicated. You should probably use “the sequencer” only once.
Context: ...alue from the DA layer and passes it to the sequencer - The sequencer uses this value to limit the size of tr...(PHRASE_REPETITION)
209-232: Refine Implementation Guidelines for consistency
Great coverage of state, errors, performance, and security. To improve readability, group related bullets (e.g., combine atomicity and state persistence) or reference the General Notes rather than repeating them.🧰 Tools
🪛 LanguageTool
[duplication] ~210-~210: Possible typo: you repeated a word.
Context: ...on environments must maintain their own state - State transitions should be atomic - State...(ENGLISH_WORD_REPEAT_RULE)
306-308: Polish neutral deployment impact bullets
Minor grammar: consider “Need to introduce a new API” for consistency.🧰 Tools
🪛 LanguageTool
[uncategorized] ~306-~306: You might be missing the article “a” here.
Context: ...yer. ### Neutral 1. Need to introduce new API exposed by rollkit. 2. Changes to e...(AI_EN_LECTOR_MISSING_DETERMINER_A)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
specs/lazy-adr/adr-010-exec-api.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
specs/lazy-adr/adr-010-exec-api.md
[uncategorized] ~24-~24: You might be missing the article “the” here.
Context: ...nterface and implement other VMs inside ABCI application. - Pros: No changes requ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~197-~197: This phrase is duplicated. You should probably use “the sequencer” only once.
Context: ...alue from the DA layer and passes it to the sequencer - The sequencer uses this value to limit the size of tr...
(PHRASE_REPETITION)
[duplication] ~210-~210: Possible typo: you repeated a word.
Context: ...on environments must maintain their own state - State transitions should be atomic - State...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~306-~306: You might be missing the article “a” here.
Context: ...yer. ### Neutral 1. Need to introduce new API exposed by rollkit. 2. Changes to e...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: rollkit-docker / docker-build (GHCR; ghcr.io/rollkit/rollkit)
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: test / Run Unit Tests
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (11)
specs/lazy-adr/adr-010-exec-api.md (11)
6-6: Update changelog with new revision date
The new entry2025.04.24: Various improvementscorrectly reflects the latest updates to the ADR.
13-21: Enhance Context section with clear execution-agnostic rationale
The added paragraph clearly frames the Execution API as a bridge to multiple VM types, improving separation of concerns and maintainability.
25-26: Add pros and cons for maintaining ABCI interface
Listing the advantages and drawbacks of keeping the ABCI interface helps readers weigh the trade-offs effectively.
28-29: Add pros and cons for migrating to Engine API
Providing explicit pros and cons for the Engine API alternative clarifies its suitability and limitations.
85-85: Ensure complete state initialization in InitChain
The new expected-behavior bullet emphasizes that all necessary state must be initialized, reinforcing robustness.
108-108: Validate transaction formatting in GetTxs
Explicitly checking that retrieved transactions are valid/formatted improves safety before passing to the sequencer.
137-137: Guarantee atomic execution in ExecuteTxs
The added bullet ensures that either all transactions succeed or none do, which is critical for state consistency.
158-160: Strengthen SetFinal guarantees
Preventing modifications to finalized blocks and optimizing storage are vital for immutability and performance.
294-295: Highlight increased flexibility and cleaner architecture
Adding these positive consequences makes the high-level benefits of the new API more explicit.
301-302: Document coordination complexity and performance trade-offs
The new negative points accurately call out the added abstraction overhead and component coordination challenges.
313-313: Add reference to go-execution repository
Linking to thego-executionrepo provides direct access to the implementation details.
Overview
Added a section on block size management to the Execution API documentation, detailing the role of the
maxBytesparameter inInitChainandExecuteTxsmethods. This update clarifies how the parameter aids in maintaining consistent block size constraints and its usage in Rollkit for validating block sizes before submission.Summary by CodeRabbit