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

"Internal return flags should remain internal" panic can be hit with inspector #1154

Closed
DaniPopes opened this issue Mar 5, 2024 · 1 comment · Fixed by #1180
Closed

"Internal return flags should remain internal" panic can be hit with inspector #1154

DaniPopes opened this issue Mar 5, 2024 · 1 comment · Fixed by #1180
Labels
good first issue Good for newcomers refactor Refactor of the code

Comments

@DaniPopes
Copy link
Collaborator

This panic

SuccessOrHalt::FatalExternalError
| SuccessOrHalt::InternalContinue
| SuccessOrHalt::InternalCallOrCreate => {
panic!("Internal return flags should remain internal {instruction_result:?}")
}

can be hit with an inspector that returns Some({Call,Create}Output { result: InstructionResult::{Continue,CallOrCreate}, .. }) in call or create.

Encountered while migrating Foundry in https://github.com/foundry-rs/foundry/pull/7125/files#diff-29a0def5856966de3e16a20140415c388f71da82a37e2cfa0308c0f76085ab9fL659-R660

I'm guessing this is fine as a panic because it can only be hit with an invalid implementation or inspector.

Perhaps a better panic message can be emitted?

@rakita
Copy link
Member

rakita commented Mar 6, 2024

Agreed with said, we should include return flag inside panic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers refactor Refactor of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants