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: check that receipt is non-nil in OnTxEnd hook #457

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

anishnaik
Copy link
Collaborator

@anishnaik anishnaik commented Aug 23, 2024

We used to store the execution trace without checking the error or whether the receipt was nil in the OnTxEnd hook. This seemed fine initially but it is possible that if the block's gas limit is reached, the receipt will be nil and error non-nil because the txn could not be fit into the block.

This PR changes the hook such that the trace is not stored if the receipt is nil or error is non-nil. The reasoning for this is that medusa will retry the failing txn in the next block and at that time we should be able to get the trace.

@anishnaik anishnaik marked this pull request as ready for review August 23, 2024 20:45
@anishnaik anishnaik changed the title throw panic if execution tracing fails Don't store trace if error is encountered Aug 23, 2024
// We avoid storing the trace for this transaction. An error should realistically only occur if we hit a block gas
// limit error. In this case, the transaction will be retried in the next block and we can retrieve the trace at
// that time.
if err != nil || receipt == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if err != nil || receipt == nil {
if receipt == nil {

I don't think we need to check both but I will merge as is

@0xalpharush 0xalpharush self-requested a review August 27, 2024 19:42
@0xalpharush 0xalpharush changed the title Don't store trace if error is encountered fix: check that receipt is non-nil in OnTxEnd hook Aug 27, 2024
@0xalpharush 0xalpharush merged commit 3beda5c into master Aug 27, 2024
12 checks passed
@0xalpharush 0xalpharush deleted the fix/execution-tracer-on-tx-end branch August 27, 2024 19:43
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.

2 participants