feat(l1): drop unwraps storage

[cdiielsi]

Motivation

Dissallow unwraps on l1.

Description

This pr drops unwraps on crate storage. Test's unwraps remain. Resolves partially #2879.

[github-actions]

Lines of code report

Total lines added: 106
Total lines removed: 0
Total lines changed: 106

Detailed view

+---------------------------------------------+-------+------+
| File                                        | Lines | Diff |
+---------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/snap.rs        | 918   | +12  |
+---------------------------------------------+-------+------+
| ethrex/crates/storage/api.rs                | 270   | +4   |
+---------------------------------------------+-------+------+
| ethrex/crates/storage/store.rs              | 1287  | +10  |
+---------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/in_memory.rs | 624   | +13  |
+---------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/libmdbx.rs   | 1406  | +37  |
+---------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/redb.rs      | 1250  | +30  |
+---------------------------------------------+-------+------+

[ilitteri]

I added comments in most of the functions that could be refactored using the transpose method, but the following are other functions where this could be done.

• In crates/storage/store_db/libmdbx.rs:
  • set_canonical_block .
  • get_payload.
  • get_pending_block.
  • get_latest_valid_ancestor ;
• In crates/storage/store_db/redb.rs:
  • get_block_hash_by_block_number .
  • get_block_header .
  • get_block_body_by_hash .
  • get_block_header_by_hash .
  • get_pending_block.
  • get_account_code .
  • get_receipt.
  • get_canonical_block_hash.
  • get_payload.
  • get_latest_valid_ancestor .

The same goes for the use of expect to help replace the use of unwraps. Using the motivation for removing the unwraps as context, it is the same to use expect in its place. I left some comments, but not in the following functions:

• In crates/storage/store_db/libmdbx.rs:
  • add_transaction_locations.
  • read_account_snapshot.
• In crates/storage/store_db/redb.rs:
  • get_transaction_location .
  • take_storage_heal_paths .
  • read_account_snapshot .

EDIT: Having read the issue, as we are allowing the use of expect for unrecoverable errors, let's add a comment saying so above the line of code that has an expect explaining in detail why the error is unrecoverable.

[SDartayet]

LGTM!

[fmoletta]

The InMemory Store variant is mostly used for local testing. I think it is overkill to modify all these trait methods just to guard the rare lock error on the in memory variant. Maybe we should just use expect and let it crash.
This is just my opinion, I would like to know what the rest of the team thinks about this

[ilitteri]

The InMemory Store variant is mostly used for local testing. I think it is overkill to modify all these trait methods just to guard the rare lock error on the in memory variant. Maybe we should just use expect and let it crash. This is just my opinion, I would like to know what the rest of the team thinks about this

I understand the appeal of using expect for simplicity in the StorageEngine implementation, as it avoids verbose error handling. However, this approach would make the in-memory Store non-compliant with the StorageEngine trait, which requires all methods to be safe and return a Result to handle errors gracefully. The inner method is central to the implementation and is called by most of all trait methods. Allowing it to panic via expect introduces inconsistency, as it undermines the trait’s guarantee of predictable error handling. While the previous use of unwrap had similar issues, we can improve by propagating errors properly with Result, ensuring the implementation aligns with the trait’s contract and maintains reliability.