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

Fix behavior on calling non-existent contracts #380

Merged
merged 3 commits into from
Aug 21, 2024
Merged

Conversation

ureeves
Copy link
Member

@ureeves ureeves commented Aug 21, 2024

The path when a contract calls another contract which does not exist was not handled, and would lead to a panic of the entire execution when it does. This PR fixes this by introducing a new error path in the c import that does not revert, and a new ContractError::DoesNotExist variant.

Eduardo Leegwater Simões added 2 commits August 21, 2024 10:51
The function is intended to test calls to contracts that do not exist,
whilst allowing for an easy check on the state.
@ureeves ureeves added the fix:bug Something isn't working label Aug 21, 2024
@ureeves ureeves requested review from miloszm and HDauven August 21, 2024 10:04
@ureeves ureeves force-pushed the contract-does-not-exist branch from 352e0a2 to 62d2e5b Compare August 21, 2024 10:05
When the contract doesn't exist it never gets pushed onto the callstack,
and instead results in a panic of the `c` import. This is clearly wrong
and this commit addresses that by introducing a new error path does not
revert the callstack.
@ureeves ureeves force-pushed the contract-does-not-exist branch from 62d2e5b to 90d80ae Compare August 21, 2024 11:18
Copy link
Contributor

@miloszm miloszm left a comment

Choose a reason for hiding this comment

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

Tested it, it fixes the issue

Copy link
Member

@HDauven HDauven left a comment

Choose a reason for hiding this comment

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

Very interesting. I seem to recall there being handling for this but it's good that we have a test now. 👍

If we can come up with more edge cases to test, even better. 😄

@ureeves ureeves merged commit 997ffbd into main Aug 21, 2024
6 checks passed
@ureeves ureeves deleted the contract-does-not-exist branch August 21, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants