Return unrecoverable error for constructor panic in nested calls

[franciszekjob]

Stack

• Return unrecoverable error for constructor panic in nested calls #3996 ⬅
• Refactor parsing panic data #3988

Closes #3992

Introduced changes

Return unrecoverable error for constructor panic in nested calls

Checklist

• Linked relevant issue
• Updated relevant documentation
• Added relevant tests
• Performed self-review of the code
• Added changes to CHANGELOG.md

[Copilot]

Pull request overview

This PR implements unrecoverable error handling for constructor panics in nested calls, aligning the behavior with Starknet's production network where deploy_syscall returns unrecoverable errors. The #[should_panic] attribute no longer catches deploy syscall failures, including constructor panics.

Key Changes:

• Introduced a new Panic enum to distinguish between constructor and entrypoint panics
• Modified error handling in CallFailure::from_execution_error to return CallFailure::Error for constructor panics instead of CallFailure::Panic
• Added comprehensive E2E tests demonstrating that constructor panics cannot be caught even with #[should_panic]

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/cheatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/panic.rs	New module defining the Panic enum to classify panic types (constructor vs entrypoint)
crates/cheatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/panic_parser.rs	Added error_contains_constructor_selector function to detect constructor-related errors
crates/cheatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/rpc.rs	Updated error handling to classify constructor panics as unrecoverable errors
crates/cheatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/mod.rs	Added the new panic module to the module hierarchy
crates/forge/tests/e2e/running.rs	Added should_panic_with_deployment test to verify new behavior with direct and proxied deployments
crates/forge/tests/data/should_panic_test/tests/should_panic_with_deployment.cairo	Test implementation demonstrating constructor panics are not catchable
crates/forge/tests/data/should_panic_test/src/constructor_panic.cairo	Contract implementations for testing constructor panic scenarios
crates/forge/tests/data/should_panic_test/src/lib.cairo	Added constructor_panic module to the library
crates/forge/tests/data/should_panic_test/tests/should_panic_test.cairo	Removed trailing whitespace
crates/forge/tests/data/should_panic_test/Scarb.toml	Added starknet and assert_macros dependencies
CHANGELOG.md	Documented the breaking change in behavior for #[should_panic] attribute

Comments suppressed due to low confidence (2)

crates/forge/tests/data/should_panic_test/tests/should_panic_with_deployment.cairo:41

• [nitpick] There's an extra blank line at the end of the file. While this doesn't affect functionality, it's inconsistent with the rest of the codebase formatting. Consider removing the extra blank line on line 41.
  crates/forge/tests/data/should_panic_test/src/constructor_panic.cairo:43
• [nitpick] There's an extra blank line at the end of the file. Consider removing the extra blank line on line 43 for consistency with the codebase formatting standards.

}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[piotmag769]

It fixes it but in NON-nested calls, nested ones were already fine it seems nvm

[cptartur]

Generally looks okay, but I'd wait with merging that before we decide on the approach for testing these kinds of failures.