[Feature]: Submit all blocks possible - #1607
Conversation
WalkthroughThe recent updates focus on enhancing the block submission logic to the Data Availability (DA) layer by refining error handling and ensuring that valid blocks are submitted even when some blocks encounter marshalling errors. This includes improvements in the Changes
Assessment against linked issues
Poem
🐰✨ 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1607 +/- ##
==========================================
+ Coverage 50.23% 52.19% +1.95%
==========================================
Files 52 52
Lines 6762 5635 -1127
==========================================
- Hits 3397 2941 -456
+ Misses 3024 2356 -668
+ Partials 341 338 -3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (3)
- block/manager_test.go (3 hunks)
- da/da.go (2 hunks)
- da/da_test.go (1 hunks)
Additional comments: 3
da/da.go (1)
- 119-149: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [122-188]
The refactoring in the
SubmitBlocksmethod improves error handling and logging for block serialization failures and blob size limits. However, consider adding more detailed comments explaining the logic, especially around the decision to break out of the loop on certain errors. This will enhance maintainability and readability for future contributors.da/da_test.go (1)
- 277-277: The updated assertion message correctly reflects the new error handling logic in the
SubmitBlocksmethod. This change ensures that test cases are aligned with the updated functionality.block/manager_test.go (1)
- 354-375: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [332-388]
The adjustments to block numbers in the test cases and the modifications related to submitting blocks to the DA layer accurately reflect the updated block submission logic. These changes ensure that the test cases are aligned with the new functionality, enhancing the reliability of the tests.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- block/manager_test.go (3 hunks)
- da/da.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- block/manager_test.go
- da/da.go
MSevey
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Some minor edits.
…decentrio/rollkit into nhan/submit_all_blocks_possible
|
hi @MSevey, can you plz re-run the CI, I think the pr look good now |
@ThanhNhann the CI ran but it failed. Can you look at the CI logs and see what the failure was? |
|
Overview
closes #1590
DAClient.SubmitBlocksso now it can submit all blocks possibleChecklist
Summary by CodeRabbit
SubmitBlocksmethod.Test_submitBlocksToDA_BlockMarshalErrorCase1andTest_submitBlocksToDA_BlockMarshalErrorCase2to reflect changes in block submission and error messaging.