Skip to content

Conversation

@blockchaindevsh
Copy link
Collaborator

It's causing all batcher transactions to fail.

Upstream also has a tiny bug here when estimating blob tx gas limit when increasing gas price, but this bug here is the primary reason all batcher txes failed for the new devnet.

@qizhou
Copy link

qizhou commented Nov 27, 2024

How did you test?

@blockchaindevsh
Copy link
Collaborator Author

blockchaindevsh commented Nov 27, 2024

I haven't tested yet, op-batcher has run out of gas now.

DL also met this problem, the fix worked for him.

This bug is introduced in this PR.

Copy link

@qizhou qizhou left a comment

Choose a reason for hiding this comment

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

The description should be updated as a simple cache mechanism is introduced to check if an address is a contract or not.

}

}
// Don't set GasLimit when inbox is contract so that later on `EstimateGas` will be called
Copy link

Choose a reason for hiding this comment

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

I feel the comments here should be updated to

Suggested change
// Don't set GasLimit when inbox is contract so that later on `EstimateGas` will be called
// Set GasLimit as intrinstic gas if the inbox is EOA, otherwise
// Leave GasLimit unset when inbox is contract so that later on `EstimateGas` will be called

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 7aab5ae.

@blockchaindevsh blockchaindevsh merged commit 577eabb into ethstorage:op-es Dec 6, 2024
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