MEL fix number of batch posting reports check bug#4464
MEL fix number of batch posting reports check bug#4464ganeshvanahalli wants to merge 2 commits intomasterfrom
Conversation
…tches extracted by MEL
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4464 +/- ##
==========================================
+ Coverage 32.92% 33.00% +0.08%
==========================================
Files 493 493
Lines 58294 58298 +4
==========================================
+ Hits 19193 19244 +51
+ Misses 35768 35685 -83
- Partials 3333 3369 +36 |
❌ 7 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
| batch.SequenceNumber, | ||
| ) | ||
| if gotHash != batchPostReportBatchHash { | ||
| continue |
There was a problem hiding this comment.
From Claude:
- No test coverage for the core bugfix scenario
File: arbnode/mel/extraction/message_extraction_function_test.go
There is no test with multiple batches where only a subset have matching reports (e.g., 3 batches, 1 report). The "OK" test uses exactly 1 batch and 1 report — still a 1:1 match. The continue path is
never exercised in a success scenario. The existing "mismatched hash" test only passes by accident because the post-loop validation fires before the panic.
- Post-loop validation (line 207) is effectively dead code for the panic case
File: arbnode/mel/extraction/message_extraction_function.go:207-213
The validation len(batchPostingReports) != batchPostReportIndex was designed to catch unprocessed reports, but due to the critical bug above, the code panics before reaching this check whenever continue
fires.
This PR fixes the bug where we previously compared the number of batch posting reports to number of batches for equality and that is not the case! As they need not be equal.
Resolves NIT-4540