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

Missing panic reason when transaction fails during account validation #286

Closed
1 task done
sgc-code opened this issue Dec 12, 2023 · 3 comments · Fixed by #287
Closed
1 task done

Missing panic reason when transaction fails during account validation #286

sgc-code opened this issue Dec 12, 2023 · 3 comments · Fixed by #287
Assignees
Labels
bug Something isn't working

Comments

@sgc-code
Copy link

Describe the bug (observed vs expected behavior)
Looks like a regression, we have multiple tests that try to assert the panic reason thrown by the account during the __validate__ method.

  • It used to work ok in the python version
  • with commit 78527decb3f76c4c808fa35f46228557af3df385 we noticed the issue in a couple of places
  • with commit 85495efb71a37ad3921c8986474b7e78a9a9f5fc we are now seeing the problem everywhere, making it impossible to run test that assert the panic reasons

To Reproduce
Send a transaction that will fail in the __validate__ method of the account. The panic reason is not present in the response

Devnet version

  • I am using Devnet version: commit 85495ef, didn't happen on commit 78527de
  • This happens with a dockerized Devnet (check the box if true).

System specifications

  • OS: MacOS
@FabijanC FabijanC added the bug Something isn't working label Dec 12, 2023
@FabijanC FabijanC self-assigned this Dec 12, 2023
@FabijanC
Copy link
Contributor

FabijanC commented Dec 12, 2023

Thanks for reporting. Can you configure your project to use a fix branch? If so, let me know how branch fix-hidden-validation-error works for you. I attempted a quick fix with #287, it might be improved tomorrow and merged.

@sgc-code
Copy link
Author

Thanks for reporting. Can you configure your project to use a fix branch? If so, let me know how branch fix-hidden-validation-error works for you. I attempted a quick fix with #287, it might be improved tomorrow and merged.

Thanks @FabijanC looks like its fixing the issue at least is all the scenarios i found so far. I couldn't run the whole test suite yet (for some reason it takes much longer to run, not sure if it's because i'm not using docker now). Thanks so much for the quick fix!

@FabijanC
Copy link
Contributor

Thanks for trying it out. The slowdown has been reported as present since #273 was merged, we're trying to figure it out. For now I can say it's most likely not related to #272 because starknet.js testing workflow takes the same amount of time with and without the old state cloning (mentioned in another place as the biggest culprit for the big memory footprint).

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in starknet-devnet-rs Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants