Skip to content

Commit

Permalink
Fix error while calling nonexistent entry point selector (#2606)
Browse files Browse the repository at this point in the history
<!-- Reference any GitHub issues resolved by this PR -->

Closes #1812

The `Err` message chosen to be thrown by `safe-dispatcher` is intended
to replicate `cairo-test` behavior. These felts are taken explicitly
from
[`CairoHintProcessor`](https://github.com/starkware-libs/cairo/blob/2ad7718591a8d2896fec2b435c509ee5a3da9fad/crates/cairo-lang-runner/src/casm_run/mod.rs#L1055-L1057)

## Introduced changes

<!-- A brief description of the changes -->

- Add proper regex to catch falling error message

## Checklist

<!-- Make sure all of these are complete -->

- [x] Linked relevant issue
- [x] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [x] Added changes to `CHANGELOG.md`
  • Loading branch information
kkawula authored Nov 4, 2024
1 parent 0e0d7e5 commit 3d8eb92
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Forge

#### Fixed

- Calling contract functions via safe-dispatcher now returns an `Err` when attempting to invoke a non-existent entry point, instead of causing a panic.

#### Changed

- You can now pass arguments to `cairo-profiler` and `cairo-coverage`. Everything after `--` will be passed to underlying binary. E.g.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ pub fn try_extract_panic_data(err: &str) -> Option<Vec<Felt252>> {
let re_string = Regex::new(r#"(?s)Execution failed\. Failure reason: "(.*?)"\."#)
.expect("Could not create string panic_data matching regex");

// CairoVM returns felts padded to 64 characters after 0x, unlike the spec's 63.
// This regex (0x[a-fA-F0-9]{0,64}) handles the padded form and is different from the spec.
let re_entry_point = Regex::new(
r"Entry point EntryPointSelector\((0x[a-fA-F0-9]{0,64})\) not found in contract\.",
)
.expect("Could not create entry point panic_data matching regex");

if let Some(captures) = re_felt_array.captures(err) {
if let Some(panic_data_match) = captures.get(1) {
let panic_data_felts: Vec<Felt252> = panic_data_match
Expand All @@ -31,6 +38,16 @@ pub fn try_extract_panic_data(err: &str) -> Option<Vec<Felt252>> {
}
}

// These felts were chosen from `CairoHintProcessor` in order to be consistent with `cairo-test`:
// https://github.com/starkware-libs/cairo/blob/2ad7718591a8d2896fec2b435c509ee5a3da9fad/crates/cairo-lang-runner/src/casm_run/mod.rs#L1055-L1057
if let Some(_captures) = re_entry_point.captures(err) {
let panic_data_felts = vec![
Felt252::from_bytes_be_slice("ENTRYPOINT_NOT_FOUND".as_bytes()),
Felt252::from_bytes_be_slice("ENTRYPOINT_FAILED".as_bytes()),
];
return Some(panic_data_felts);
}

None
}

Expand Down
18 changes: 18 additions & 0 deletions crates/forge/tests/data/nonexistent_selector/Scarb.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[package]
name = "nonexistent_selector"
version = "0.1.0"
edition = "2023_11"

# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html

[dependencies]
starknet = "2.4.0"

[dev-dependencies]
snforge_std = { path = "../../../../../snforge_std" }

[[target.starknet-contract]]
sierra = true

[scripts]
test = "snforge test"
24 changes: 24 additions & 0 deletions crates/forge/tests/data/nonexistent_selector/src/lib.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#[starknet::interface]
pub trait IMyContract<TState> {
fn my_function(self: @TState);
}

#[starknet::contract]
pub mod MyContract {
use starknet::SyscallResultTrait;
use starknet::syscalls::call_contract_syscall;

#[storage]
pub struct Storage {}

#[abi(embed_v0)]
impl MyContract of super::IMyContract<ContractState> {
fn my_function(self: @ContractState) {
let this = starknet::get_contract_address();
let selector = selector!("nonexistent");
let calldata = array![].span();

call_contract_syscall(this, selector, calldata).unwrap_syscall();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use starknet::ContractAddress;

use snforge_std::{declare, ContractClassTrait, DeclareResultTrait};

use nonexistent_selector::IMyContractSafeDispatcher;
use nonexistent_selector::IMyContractSafeDispatcherTrait;

#[test]
#[feature("safe_dispatcher")]
fn test_unwrapped_call_contract_syscall() {
let contract = declare("MyContract").unwrap().contract_class();
let (contract_address, _) = contract.deploy(@array![]).unwrap();

let safe_dispatcher = IMyContractSafeDispatcher { contract_address };
let res = safe_dispatcher.my_function();
match res {
Result::Ok(_) => panic!("Expected an error"),
Result::Err(err_data) => {
assert(*err_data.at(0) == 'ENTRYPOINT_NOT_FOUND', *err_data.at(0));
assert(*err_data.at(1) == 'ENTRYPOINT_FAILED', *err_data.at(1));
},
};
}
18 changes: 18 additions & 0 deletions crates/forge/tests/e2e/running.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1100,3 +1100,21 @@ fn catch_runtime_errors() {
),
);
}

#[test]
fn call_nonexistent_selector() {
let temp = setup_package("nonexistent_selector");

let output = test_runner(&temp).assert().code(0);

assert_stdout_contains(
output,
indoc! {r"
Collected 1 test(s) from nonexistent_selector package
Running 1 test(s) from tests/
[PASS] nonexistent_selector_integrationtest::test_contract::test_unwrapped_call_contract_syscall (gas: ~103)
Running 0 test(s) from src/
Tests: 1 passed, 0 failed, 0 skipped, 0 ignored, 0 filtered out
"},
);
}
2 changes: 1 addition & 1 deletion crates/forge/tests/integration/dispatchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ fn nonexistent_libcall_function() {
assert_case_output_contains(
&result,
"nonexistent_libcall_function",
"Entry point EntryPointSelector(0x1fdb214e1495025fa4baf660d34f03c0d8b5037cf10311d2a3202a806aa9485) not found in contract"
"(0x454e545259504f494e545f4e4f545f464f554e44 ('ENTRYPOINT_NOT_FOUND'), 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'))"
);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/forge/tests/integration/test_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,8 @@ fn cant_call_test_contract() {
let result = run_test_case(&test);

assert_failed(&result);
assert_case_output_contains(&result, "cant_call_test_contract", "Entry point");
assert_case_output_contains(&result, "cant_call_test_contract", "not found in contract");
assert_case_output_contains(&result, "cant_call_test_contract", "ENTRYPOINT_NOT_FOUND");
assert_case_output_contains(&result, "cant_call_test_contract", "ENTRYPOINT_FAILED");
}

#[test]
Expand Down

0 comments on commit 3d8eb92

Please sign in to comment.