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

feat: improves error messaging for better debugging #468

Closed
wants to merge 7 commits into from

Conversation

dutterbutter
Copy link
Collaborator

@dutterbutter dutterbutter commented Dec 3, 2024

What 💻

Why ✋

  • Previous error messages were confusing / did not provide insightful messages

Evidence 📷

Contract reversion error:
Screenshot 2024-12-03 at 12 34 29 PM

Paymaster error:
Screenshot 2024-12-03 at 12 35 01 PM

Function selector error:
Screenshot 2024-12-03 at 12 35 21 PM

Comment on lines +801 to +802
// TODO: Ask if vm.inspect is the same as vm.execute
let tx_result = vm.inspect(&mut tracers.into(), InspectExecutionMode::OneTx);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itegulov do you know if calling vm.inspect vs vm.execute makes any difference? It doesnt seem so but wanted to confirm.

Comment on lines +743 to +748
) -> (
VmExecutionResultAndLogs,
Vec<Call>,
ErrorFlags,
BootloaderDebug,
) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont love returning this tuple from here, but unsure how else to make it work

@ly0va
Copy link
Member

ly0va commented Dec 3, 2024

Very cool!

Can we make data be displayed in hex instead of Uint8Array? I think it's a bit more concise and readable that way.

Also, controversial, but not all terminals support emoji. Can we find an ascii/utf8 replacement for 🔴 ?

@dutterbutter
Copy link
Collaborator Author

Very cool!

Can we make data be displayed in hex instead of Uint8Array? I think it's a bit more concise and readable that way.

Also, controversial, but not all terminals support emoji. Can we find an ascii/utf8 replacement for 🔴 ?

Thanks for the feedback! Yes, certainly.

Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the massive amount of conflicts, completely missed this PR! I think this is still useful and lmk if you need help with rebasing the changes on the new project structure.

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

Successfully merging this pull request may close these issues.

3 participants