Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

L2: integrate the no_panic crate #1369

Open
fborello-lambda opened this issue Dec 2, 2024 · 0 comments
Open

L2: integrate the no_panic crate #1369

fborello-lambda opened this issue Dec 2, 2024 · 0 comments
Assignees
Labels

Comments

@fborello-lambda
Copy link
Contributor

The no_panic crate detects if a function panics in the linking process.

Things to consider:

  • async functions aren't compatible with the#[no_panic] macro.
    Use case Example:
    In crates/l2/l1_committer.rs
use no_panic::no_panic;
//[...]
    #[no_panic]
    pub fn get_deposit_hash(&self, deposit_hashes: Vec<H256>) -> Result<H256, CommitterError> {
        let s = "\u{1f980}input string";
        let c = &s[1..];
        println!("{c}");
//[...]

Then if we build the program:

cargo build --release --manifest-path ../../Cargo.toml --bin ethrex --features l2

it should output:

 ERROR[no-panic]: detected panic in function `get_deposit_hash`

Task to perform: Ideally, we should use the #[no_panic] macro in every non-async function. Then fix all the compilation errors.

Note

For the async functions, we are handling the errors with a loop for each component (the run function). We may want a catch_unwind as follows:

fn safe_execute<F: FnOnce() -> T, T>(f: F) -> Result<T, Box<dyn std::any::Any + Send>> {
    panic::catch_unwind(panic::AssertUnwindSafe(f))
}

fn main() {
    let result = safe_execute(|| {
        let v = vec![1, 2, 3];
        v[5] // This panics
    });

    match result {
        Ok(_) => (),
        Err(e) => error!("CATCH panic {e:?}"),
    }
}
github-merge-queue bot pushed a commit that referenced this issue Dec 10, 2024
**Motivation**

As explained by issue #1369, implementing the `no_panic` crate would
help prevent some hidden panics.

**Description**

I've tried to implement it for the non-async functions, facing the
following problems:
- For functions interacting with `TcpStream` and the `Database` it
throws a "panic compilation error".
- The `keccak` function used and `c-kzg` throws the "panic compilation
error".
- `no_panic` seems to be a crate that aims to avoid panics in low level
lib crates.

- Found a potential panic in `fake_exponential`, it's solved by the
`fake_exponential_checked` implementation.
- Some extra changes regarding usability were added.
- Remove `pub` keyword if not needed.

See if we can close the issue #1369
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Status: No status
Development

No branches or pull requests

1 participant