fix(supervisor/rpc): localUnsafe always points to genesis#2145
fix(supervisor/rpc): localUnsafe always points to genesis#2145dhyaniarun1993 merged 7 commits intoop-rs:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with the localUnsafe pointer by updating the safety head reference to the correct block rather than defaulting to genesis. The key changes include:
- Updating the state manager with the safety head reference using SafetyLevel::Unsafe.
- Adding corresponding error logging for failures.
- Including a new test case to validate the update of the safety head reference.
Comments suppressed due to low confidence (1)
crates/supervisor/core/src/chain_processor/task.rs:238
- [nitpick] The use of an underscore prefix in '_safety_level' typically indicates an unused variable. Since it is actively being compared, consider renaming it to 'safety_level' for clarity.
assert_eq!(_safety_level, SafetyLevel::Unsafe);
| chain_id = self.chain_id, | ||
| block_number = block_info.number, | ||
| %err, | ||
| "Failed to store block reference" |
There was a problem hiding this comment.
The error message 'Failed to store block reference' may be confusing since the call is updating the safety head reference. Consider revising the message to better reflect the context of the update.
| "Failed to store block reference" | |
| "Failed to update safety head reference" |
|
thanks @varun-doshi ! lint-native check remaining then ready for review |
emhane
left a comment
There was a problem hiding this comment.
nice! pending @dhyaniarun1993
dhyaniarun1993
left a comment
There was a problem hiding this comment.
Thanks @varun-doshi for picking this up. The Safety head ref should be done within the same transaction as process_and_store_logs. Please refer save_derived_block_pair.
| "Processing unsafe block" | ||
| ); | ||
| if let Err(err) = self.log_indexer.process_and_store_logs(&block_info).await { | ||
| let log_entries = match self.log_indexer.process_logs(&block_info).await { |
There was a problem hiding this comment.
I think we can delegate the responsibility of saving data to process_and_store_logs of the logIndexer since it's already using the logWriters store_block_logs which in-turns updates the safety head ref. @sadiq1971 What do you think?
There was a problem hiding this comment.
Yes @dhyaniarun1993. The tasks of the log_indexer are both - process and store the logs.
|
@varun-doshi Can you resolve the conflicts? |
…#2145) Ref op-rs/kona#2136 --------- Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
Ref #2136