feat(rollup-relayer): add sanity checks before committing and finalizing #1714
Conversation
WalkthroughThis update introduces comprehensive sanity checks and validation routines for Layer 2 batch and chunk data processing in the relayer logic. It adds a new file with detailed validation functions, integrates these checks into batch processing and transaction construction, and updates related tests to ensure correct parent hash assignment and block numbering. Additionally, a database method for batch lookup by hash is added. Changes
Sequence Diagram(s)sequenceDiagram
participant Test/Relayer
participant Layer2Relayer
participant SanityChecks
participant Database
Test/Relayer->>Layer2Relayer: ProcessPendingBatches()
Layer2Relayer->>SanityChecks: sanityChecksBeforeConstructingTransaction(batches)
SanityChecks->>Database: getPreviousChunkForContinuity()
SanityChecks-->>Layer2Relayer: Validation result (error or success)
alt Validation fails
Layer2Relayer-->>Test/Relayer: Abort with error
else Validation passes
Layer2Relayer->>Layer2Relayer: constructCommitBatchPayload...
Layer2Relayer->>Layer2Relayer: Submit transaction
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
common/version/version.go(1 hunks)rollup/internal/controller/relayer/l2_relayer.go(10 hunks)rollup/internal/orm/batch.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
Learnt from: colinlyguo
PR: #1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In batch_proposer.go, it's acceptable to call utils.CalculateBatchMetrics multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
🔇 Additional comments (6)
rollup/internal/orm/batch.go (1)
266-278: LGTM!The
GetBatchByHashmethod implementation follows the established pattern used byGetBatchByIndexand other ORM methods in this file. Error handling and return values are consistent.rollup/internal/controller/relayer/l2_relayer.go (4)
476-481: Good placement of sanity checks!The sanity checks are appropriately placed after batch selection but before transaction construction. This ensures invalid batches are caught early without wasting resources on payload construction.
950-1074: Comprehensive validation in payload construction!The added sanity checks provide excellent coverage:
- Nil pointer protection for inputs
- Batch index consistency validation
- Block count verification
- Hash field validation (preventing zero hashes)
- L1 message queue hash consistency checks
- Generated payload validation
These checks will prevent malformed transactions from being submitted to L1.
1398-1427: Well-structured validation architecture!The validation flow is logical and comprehensive:
- Basic nil/empty checks
- Codec version consistency
- Continuity validation with previous chunks
- Detailed batch and chunk validation
The separation of concerns makes the code maintainable and easy to understand.
1077-1128: Consistent validation across all payload construction methods!The sanity checks in validium-specific methods maintain the same high standard:
- Nil input validation
- Empty data checks
- Zero hash prevention
- Consistent error messaging
This ensures both regular and validium modes have robust validation.
Also applies to: 1131-1188, 1191-1231
common/version/version.go (1)
8-8: Version bump looks good.The version increment from v4.5.35 to v4.5.36 appropriately reflects the addition of sanity checks in the rollup-relayer.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
1548-1555: Parent batch validation correctly implements strict checking.As previously discussed and confirmed by the author, the function correctly errors out when the parent batch is not found, treating it as a genuine error condition rather than attempting graceful handling.
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
1637-1646: Consider adding comments to clarify previous chunk selection logic.The logic for selecting the previous chunk handles three distinct cases correctly, but would benefit from explicit documentation.
// Get previous chunk for continuity check var prevChunk *orm.Chunk if chunkIndex > 0 { + // Case 1: Not the first chunk in batch - use previous chunk from same batch prevChunk = batch.Chunks[chunkIndex-1] } else if i == 0 { + // Case 2: First chunk of first batch - use the provided previous chunk from previous batch prevChunk = prevChunkFromPrevBatch } else if i > 0 { - // Use the last chunk from the previous batch + // Case 3: First chunk of subsequent batch - use the last chunk from the previous batch prevBatch := allBatches[i-1] prevChunk = prevBatch.Chunks[len(prevBatch.Chunks)-1] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rollup/internal/controller/relayer/l2_relayer.go(12 hunks)rollup/internal/controller/relayer/relayer_test.go(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
rollup/internal/controller/relayer/relayer_test.go (1)
Learnt from: colinlyguo
PR: #1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
rollup/internal/controller/relayer/l2_relayer.go (2)
Learnt from: colinlyguo
PR: #1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
Learnt from: colinlyguo
PR: #1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In batch_proposer.go, it's acceptable to call utils.CalculateBatchMetrics multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
🧬 Code Graph Analysis (1)
rollup/internal/controller/relayer/l2_relayer.go (2)
rollup/internal/orm/batch.go (2)
Batch(22-72)Batch(80-82)rollup/internal/orm/chunk.go (2)
Chunk(20-64)Chunk(72-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
🔇 Additional comments (7)
rollup/internal/controller/relayer/relayer_test.go (1)
84-84: Test setup properly configured for block number validation.The explicit assignment of block numbers ensures test blocks have the required metadata for the new sanity checks that validate block continuity and ranges.
Also applies to: 98-98
rollup/internal/controller/relayer/l2_relayer.go (6)
280-292: Good addition of genesis batch validation checks.The sanity checks properly validate critical fields before committing the genesis batch, preventing invalid transactions from being sent to L1.
Also applies to: 312-316
494-499: Sanity checks appropriately placed before transaction construction.The validation prevents invalid batches from progressing to transaction construction, failing fast with detailed error logging.
968-1090: Comprehensive validation in payload construction.The extensive checks cover all critical aspects:
- Nil safety and index consistency
- Block presence and count verification
- Parent batch hash validation
- L1 message queue hash consistency with detailed edge case handling
- Blob and calldata validation
These validations effectively prevent malformed payloads from being constructed.
1095-1148: Validium payload construction properly validated.The checks ensure critical validium fields are present, including the commitment hash from the last chunk's end block hash.
1154-1217: Finalize bundle payload validation is thorough.Both codec versions properly validate essential fields before finalization, with appropriate differences (e.g., state root check only for CodecV7).
Also applies to: 1219-1258
1427-1456: Well-structured validation architecture.The hierarchical approach with clear separation of concerns makes the validation logic maintainable and easy to understand. The flow from basic to detailed validation is logical.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1714 +/- ##
==========================================
Coverage ? 37.56%
==========================================
Files ? 243
Lines ? 20437
Branches ? 0
==========================================
Hits ? 7678
Misses ? 11952
Partials ? 807
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…committing-and-finalizing
…committing-and-finalizing
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
77bba2d to
450af64
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer_sanity.go (1)
251-339: Complex but critical validation function - consider refactoring for maintainability.The function implements comprehensive calldata and blob validation for codec v7, with proper ABI decoding, blob decoding, and thorough comparison with database data. The validation logic is sound and covers all critical aspects.
However, the function is quite long (88 lines) and handles multiple concerns. Consider breaking it down into smaller functions for better maintainability:
+// validateCalldataParams validates the decoded calldata parameters +func (r *Layer2Relayer) validateCalldataParams(version uint8, parentBatchHash, lastBatchHash common.Hash, firstBatch, lastBatch *orm.Batch) error { + // Move lines 277-285 here +} + +// validateBlobPayload validates a single blob payload against database data +func (r *Layer2Relayer) validateBlobPayload(blob *kzg4844.Blob, dbBatch *dbBatchWithChunks, codec encoding.Codec) error { + // Move lines 298-334 here +}This would make the main function more readable and each helper function more focused on a single responsibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rollup/internal/controller/relayer/l2_relayer.go(12 hunks)rollup/internal/controller/relayer/l2_relayer_sanity.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rollup/internal/controller/relayer/l2_relayer.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
📚 Learning: in rollup/internal/controller/relayer/l2_relayer.go, the validatebatchfields function should error o...
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
Applied to files:
rollup/internal/controller/relayer/l2_relayer_sanity.go
📚 Learning: in `batch_proposer.go`, it's acceptable to call `utils.calculatebatchmetrics` multiple times within ...
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
Applied to files:
rollup/internal/controller/relayer/l2_relayer_sanity.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
🔇 Additional comments (8)
rollup/internal/controller/relayer/l2_relayer_sanity.go (8)
14-50: LGTM! Well-structured message queue validation.The function correctly validates L1 message queue hash consistency with proper handling of genesis batches, empty chunks, and appropriate zero-hash checks. The logic for ensuring hash differences when messages are processed is sound.
52-71: Function structure is solid.The sanity check orchestration is well-organized with proper error handling and sequential validation steps.
73-86: LGTM! Proper continuity validation setup.The function correctly identifies genesis chunks as inappropriate for normal batch submission flow and handles database retrieval with comprehensive error context.
88-118: LGTM! Comprehensive batch validation pipeline.The function implements a well-structured validation pipeline with proper fail-fast basic checks, codec version consistency validation, and sequential detailed validation with appropriate state tracking.
120-138: LGTM! Well-organized validation orchestration.The function properly delegates to specialized validation functions in a logical order, ensuring comprehensive batch validation with appropriate error propagation.
140-180: LGTM! Critical batch field validation implemented correctly.The function properly validates essential batch fields including hash integrity, sequential indexing, and parent batch continuity. The database lookup for parent batch validation ensures blockchain integrity, which aligns with the requirement that missing parent batches should error out as genuine error conditions.
182-200: LGTM! Proper chunk validation with consistency checks.The function correctly validates codec version consistency between chunks and their parent batch, and implements sequential chunk validation with appropriate state tracking for continuity checks.
202-249: LGTM! Comprehensive chunk validation with continuity checks.The function implements thorough validation covering chunk integrity, sequential indexing, L1 message continuity, block range validation, and cross-chunk continuity. The L1 messages popped continuity calculation is particularly important for maintaining L1-L2 message processing integrity.
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
450af64 to
925ef74
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer_sanity.go (1)
251-350: Consider optimizing database queries for large batches.The function performs comprehensive validation of calldata and blobs, which is excellent. However, there's a potential performance concern: for each batch, it fetches all blocks from the database (lines 312-317). For large batches with many blocks, this could be expensive.
Consider batching the block retrieval or implementing a more efficient query strategy if performance becomes an issue.
// Collect all blocks for the batch var batchBlocks []*encoding.Block -for _, c := range dbChunks { - blocks, err := r.l2BlockOrm.GetL2BlocksInRange(r.ctx, c.StartBlockNumber, c.EndBlockNumber) - if err != nil { - return fmt.Errorf("failed to get blocks for batch %d chunk %d: %w", dbBatch.Index, c.Index, err) - } - batchBlocks = append(batchBlocks, blocks...) -} +// Collect block ranges first, then fetch in a single query if possible +startBlock := dbChunks[0].StartBlockNumber +endBlock := dbChunks[len(dbChunks)-1].EndBlockNumber +blocks, err := r.l2BlockOrm.GetL2BlocksInRange(r.ctx, startBlock, endBlock) +if err != nil { + return fmt.Errorf("failed to get blocks for batch %d: %w", dbBatch.Index, err) +} +batchBlocks = blocks
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rollup/internal/controller/relayer/l2_relayer.go(12 hunks)rollup/internal/controller/relayer/l2_relayer_sanity.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rollup/internal/controller/relayer/l2_relayer.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
📚 Learning: in rollup/internal/controller/relayer/l2_relayer.go, the validatebatchfields function should error o...
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
Applied to files:
rollup/internal/controller/relayer/l2_relayer_sanity.go
📚 Learning: in `batch_proposer.go`, it's acceptable to call `utils.calculatebatchmetrics` multiple times within ...
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
Applied to files:
rollup/internal/controller/relayer/l2_relayer_sanity.go
🔇 Additional comments (8)
rollup/internal/controller/relayer/l2_relayer_sanity.go (8)
15-50: LGTM! Comprehensive L1 message queue validation.The function correctly handles all validation scenarios including genesis batch handling, empty chunk validation, and hash consistency checks. The logic ensures proper L1 message queue state transitions.
53-71: LGTM! Well-structured sanity check orchestration.The function properly orchestrates the sanity check workflow with appropriate error handling and logging for operational visibility.
74-86: LGTM! Proper continuity check implementation.The function correctly prevents genesis chunks from normal batch submission flow and retrieves the previous chunk with appropriate error handling.
89-118: LGTM! Comprehensive batch validation with proper state tracking.The function correctly combines basic validation, codec version consistency checks, and detailed validation while maintaining proper chunk continuity state throughout the process.
121-138: LGTM! Well-organized single batch validation.The function properly delegates to specific validation functions, maintaining good separation of concerns and making the code maintainable.
141-180: LGTM! Thorough batch field validation with proper parent batch handling.The function correctly validates all essential batch fields and properly handles parent batch continuity validation through database lookup, which aligns with the expected behavior for missing parent batches.
183-200: LGTM! Proper chunk validation with codec consistency checks.The function correctly validates codec version consistency and maintains proper chunk continuity state while providing clear error context.
203-249: LGTM! Comprehensive single chunk validation.The function correctly validates all essential chunk fields including hash values, sequential indices, L1 message continuity, block ranges, and inter-chunk continuity.
colinlyguo
changed the title
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
39fb299 to
d3720a4
Compare
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
d3720a4 to
f22bd44
Compare
colinlyguo
changed the title
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
f22bd44 to
52cf05c
Compare
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
52cf05c to
cf85225
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
rollup/internal/controller/relayer/l2_relayer_sanity.go (1)
1-439: Address test coverage gap for critical validation functions.Based on the coverage report, this file has only 34.72% patch coverage with 126 lines missing coverage. Given that these are critical sanity check functions that validate transaction data integrity, comprehensive test coverage is essential to ensure the validation logic works correctly and catches edge cases.
The low test coverage is concerning for such critical validation logic. Would you like me to help generate comprehensive unit tests for these sanity check functions to improve coverage and ensure reliability?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rollup/internal/controller/relayer/l2_relayer_sanity.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
📚 Learning: in rollup/internal/controller/relayer/l2_relayer.go, the validatebatchfields function should error o...
Learnt from: colinlyguo
PR: scroll-tech/scroll#1714
File: rollup/internal/controller/relayer/l2_relayer.go:1548-1555
Timestamp: 2025-07-29T16:38:24.647Z
Learning: In rollup/internal/controller/relayer/l2_relayer.go, the validateBatchFields function should error out when GetBatchByHash fails to find a parent batch. This is intentional behavior - missing parent batches represent genuine error conditions that should halt batch submission processing. Genesis batch handling occurs separately from normal batch validation flow.
Applied to files:
rollup/internal/controller/relayer/l2_relayer_sanity.go
📚 Learning: in `batch_proposer.go`, it's acceptable to call `utils.calculatebatchmetrics` multiple times within ...
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
Applied to files:
rollup/internal/controller/relayer/l2_relayer_sanity.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
🔇 Additional comments (10)
rollup/internal/controller/relayer/l2_relayer_sanity.go (10)
16-48: LGTM! Well-structured orchestration function with proper error handling.The main validation function follows a clear, layered approach with appropriate error wrapping and early returns. The validation flow is logical and comprehensive.
58-91: LGTM! Robust calldata parsing with comprehensive type assertions.The function properly handles ABI method unpacking and includes thorough type assertions for all parameters. Error messages are descriptive and actionable.
142-159: LGTM! Thorough validation of calldata against database records.The validation correctly compares calldata parameters with database records, ensuring consistency between constructed transaction data and stored state.
162-203: LGTM! Comprehensive database consistency validation with proper error handling.The function performs thorough consistency checks across batches and chunks, including codec version uniformity and proper chunk continuity validation. The logic for handling the previous chunk correctly avoids the genesis chunk scenario.
206-258: LGTM! Thorough single batch validation with proper parent batch verification.The validation correctly checks all essential fields and handles batch index continuity. The special handling for the first batch (lines 242-250) properly validates against the parent batch from the database, which aligns with the retrieved learning about intentional error behavior when parent batches are missing.
261-279: LGTM! Proper chunk-batch consistency validation.The function correctly validates codec version consistency between chunks and their parent batch, and properly validates each chunk individually with continuity checks.
282-329: LGTM! Comprehensive single chunk validation with proper continuity checks.The validation thoroughly checks chunk hash fields, block ranges, and L1 message continuity. The logic correctly ensures chunks are sequential and properly linked with their predecessors.
332-351: LGTM! Proper blob validation orchestration.The function correctly retrieves the appropriate codec and validates each blob against its corresponding batch data with proper error handling.
354-404: LGTM! Thorough blob-to-batch validation with comprehensive checks.The function properly:
- Collects all blocks for the batch from multiple chunks
- Validates block counts match expected ranges
- Decodes blob payload using the correct codec
- Validates L1 message queue hashes match database records
- Ensures decoded block data matches database block data
407-438: LGTM! Robust L1 message queue consistency validation.The function performs comprehensive validation of L1 message queue hashes, correctly:
- Validating that hashes are non-zero when messages have been processed
- Ensuring prev and post hashes differ when messages are processed in the batch
- Calculating total L1 messages processed correctly
This addresses potential issues with incorrect L1 message queue hash computation mentioned in past review comments.
colinlyguo
force-pushed
the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
from
f4bc536 to
3f9d9cc
Compare
colinlyguo
deleted the
feat-rollup-relayer-add-sanity-checks-in-committing-and-finalizing
branch
Purpose or design rationale of this PR
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tagincommon/version.gobeen updated or have you addedbump-versionlabel to this PR?Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores