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: Notify user of tx fail in boa call output #188

Merged
merged 5 commits into from
Jun 19, 2024
Merged

Conversation

DanielSchiavini
Copy link
Collaborator

Closes #138

What I did

  • Check whether the transaction receipt has a success status

How I did it

How to verify it

  • Test is included

Description for the changelog

  • Notify user if the transaction receipt returns a failed status

Cute Animal Picture

image

boa/network.py Outdated
@@ -419,6 +419,8 @@ def _send_txn(self, from_, to=None, gas=None, value=None, data=None):
print(f"tx broadcasted: {tx_hash}")

receipt = self._rpc.wait_for_tx_receipt(tx_hash, self.tx_settings.poll_timeout)
if receipt["status"] != "0x1":
raise RuntimeError(f"txn failed: {receipt}")
Copy link
Member

Choose a reason for hiding this comment

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

why RuntimeError?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BoaError expects a stack trace. Which should we use?

Copy link
Member

Choose a reason for hiding this comment

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

RuntimeError might be ok, but i'm not sure. i think the only way we can reach here is if the fork mode simulation did not raise any errors, but there is an error when actually sending the transaction (presumably caused by a state change between our snapshot and now).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

@charles-cooper charles-cooper Apr 11, 2024

Choose a reason for hiding this comment

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

hmm, i'm a bit confused why it's not already being caught in one of these two places:

if trace.is_error and not computation.is_error:

or
if local_address != create_address:

@DanielSchiavini
Copy link
Collaborator Author

DanielSchiavini commented Apr 25, 2024

@charles-cooper closed #138 as completed in e3c4e9c 2 weeks ago

@DanielSchiavini DanielSchiavini deleted the 138/notify branch April 25, 2024 07:10
@charles-cooper charles-cooper restored the 138/notify branch May 8, 2024 11:32
@charles-cooper
Copy link
Member

it was actually reverted in the same PR - aca2926

@charles-cooper charles-cooper self-requested a review June 19, 2024 14:18
@charles-cooper charles-cooper merged commit ea18746 into master Jun 19, 2024
14 checks passed
@DanielSchiavini DanielSchiavini deleted the 138/notify branch September 11, 2024 14:17
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.

Notify user of tx fail in boa call output
2 participants