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

Improve known cheat code errors #497

Open
arcz opened this issue Jun 21, 2024 · 2 comments · May be fixed by #503
Open

Improve known cheat code errors #497

arcz opened this issue Jun 21, 2024 · 2 comments · May be fixed by #503
Labels

Comments

@arcz
Copy link
Collaborator

arcz commented Jun 21, 2024

We return non-helpful errors when there is a problem with a known cheat code. For instance, here we know we are inside ffi but we return BadCheatCode with sig which doesn't add much debugging value.

Ideally, in the error, we should have the method + args like ffi(string[]) and a failure reason.

@msooseth msooseth linked a pull request Jul 15, 2024 that will close this issue
4 tasks
@msooseth
Copy link
Collaborator

Hi @arcz,

Thanks for this! I have done some work on this, but for the life of me I can't find a way to test it 😆 Please see PR #503

There, the file test/contracts/pass/cheatCodeErrors.sol is what's supposed to trigger the error. However, it seems like the interpreter ignores that there are too few arguments (1 instead of 2). I can't seem to make the interpreter die. If you change the 0x24 to 0x44 there, the hevm.deal works, so indeed it is a correct call.

So I'm stuck at testing my new fix. Do you know how could I write that inline assembly such that the interpreter will die with a BadCheatcode and I could test my error code? On bad selector, it reverts, on incorrect parameters it just runs happily ever after, I'm confused how to actually trigger one of these 😆

Thanks,

Mate

@msooseth
Copy link
Collaborator

Blocked currently on a good test case and a follow-up review. Perhaps in September when fewer people are on vacation, this will be merged :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants