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

Panics not caught in a nested contracts #1627

Closed
1 task done
delaaxe opened this issue Jan 30, 2024 · 4 comments · Fixed by #1679
Closed
1 task done

Panics not caught in a nested contracts #1627

delaaxe opened this issue Jan 30, 2024 · 4 comments · Fixed by #1679
Assignees
Labels
bug Something isn't working snforge

Comments

@delaaxe
Copy link

delaaxe commented Jan 30, 2024

Which component is your bug related to?

snforge

Foundry Version

0.16

What operating system are you using?

MacOS

What system architecture are you using?

x86

What happened

use starknet::ContractAddress;

#[starknet::interface]
trait IContractA<TContractState> {
    fn do_panic(self: @TContractState);
}

#[starknet::contract]
mod ContractA {
    #[storage]
    struct Storage {}

    #[abi(embed_v0)]
    impl Impl of super::IContractA<ContractState> {
        fn do_panic(self: @ContractState) {
            assert!(false, "foo");
        }
    }
}

#[starknet::interface]
trait IContractB<TContractState> {
    fn call_contractA(self: @TContractState, contract_address: ContractAddress);
}

#[starknet::contract]
mod ContractB {
    use starknet::ContractAddress;
    use super::{IContractADispatcherTrait, IContractADispatcher};

    #[storage]
    struct Storage {}

    #[abi(embed_v0)]
    impl Impl of super::IContractB<ContractState> {
        fn call_contractA(self: @ContractState, contract_address: ContractAddress) {
            IContractADispatcher { contract_address }.do_panic();
        }
    }
}

#[cfg(test)]
mod TestReverts {
    use snforge_std::{declare, ContractClassTrait};
    use super::{IContractADispatcherTrait, IContractADispatcher, IContractBDispatcherTrait, IContractBDispatcher};

    #[test]
    #[should_panic(expected: "foo")]
    fn test_should_panic_1() {
        let contractA_address = declare('ContractA').deploy(@array![]).unwrap();
        let contractA = IContractADispatcher { contract_address: contractA_address };
        contractA.do_panic();
    }

    #[test]
    #[should_panic(expected: "foo")]
    fn test_should_panic_2() {
        let contractA_address = declare('ContractA').deploy(@array![]).unwrap();
        let contractB_address = declare('ContractB').deploy(@array![]).unwrap();
        let contractB = IContractBDispatcher { contract_address: contractB_address };
        contractB.call_contractA(contractA_address);
    }
}

Trace

Screenshot 2024-01-30 at 20 38 11

Is there an existing issue for this?

  • I have searched the existing issues and verified no issue exits for this problem.
@delaaxe delaaxe added the bug Something isn't working label Jan 30, 2024
@github-actions github-actions bot added the new label Jan 30, 2024
@delaaxe delaaxe changed the title Foundy can't catch a panic happening in Foundy can't catch a panic happening in a nested contract Jan 30, 2024
@delaaxe delaaxe changed the title Foundy can't catch a panic happening in a nested contract No catching of panic happening in a nested contract Jan 30, 2024
@delaaxe delaaxe changed the title No catching of panic happening in a nested contract Panics not caught in a nested contracts Jan 30, 2024
@drknzz drknzz self-assigned this Jan 31, 2024
@drknzz drknzz moved this from Triage to TODO in Starknet foundry Jan 31, 2024
@drknzz drknzz added the snforge label Jan 31, 2024
@drknzz
Copy link
Contributor

drknzz commented Jan 31, 2024

Hey @delaaxe, snforge hasn't migrated to strings yet, so the example you provided should work if you change it to this:

#[starknet::contract]
mod ContractA {
    #[storage]
    struct Storage {}

    #[abi(embed_v0)]
    impl Impl of super::IContractA<ContractState> {
        fn do_panic(self: @ContractState) {
            assert(false, 'foo');
        }
    }
}

#[test]
#[should_panic(expected: ('foo',))]
fn test_should_panic_2() {
...

@drknzz drknzz removed their assignment Jan 31, 2024
@drknzz
Copy link
Contributor

drknzz commented Jan 31, 2024

Connected #1507

@delaaxe
Copy link
Author

delaaxe commented Jan 31, 2024 via email

@piotmag769
Copy link
Member

This is a bug, related to #1344: we need to explore lower level methods to retrieve panic data from blockifier

@Arcticae Arcticae moved this from TODO to In Progress in Starknet foundry Feb 2, 2024
@Arcticae Arcticae self-assigned this Feb 2, 2024
@Arcticae Arcticae removed the new label Feb 2, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 12, 2024
<!-- Reference any GitHub issues resolved by this PR -->

Closes #1627 

## Introduced changes

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

- Adds a mechanism for parsing the errors from the trace and passing it
on to test runner
- Adds an extension for ease of further parsing the array

## 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`

---------

Co-authored-by: Piotr Magiera <[email protected]>
Co-authored-by: Kamil Jankowski <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Starknet foundry Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working snforge
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants