-
Notifications
You must be signed in to change notification settings - Fork 277
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: eth_estimateGas error in zero balance address #537
Conversation
I am not sure whether it's what we want, if a user estimates gas and then uses this gas to call, the call will still fail? i feel like we can handle this types of error specially (if it's this error type, don't return error in |
It works at my local env. |
L1 data fee is |
Local test looks good (
|
One situation that should be considered is that l1DataFee is used when running but |
More complete verification is needed. |
11bf5a9
An additional note:
go-ethereum/internal/ethapi/api.go Lines 1099 to 1105 in 11bf5a9
This is consistent with the way that geth handles |
Yeah. Let's consider how to handle this systematically after receiving more feedback. Potential inconsistency with SDKs. |
1. Purpose or design rationale of this PR
Correct response (Ethereum, account with 0 balance):
Incorrect response (Scroll, account with 0 balance):
Root cause:
a. https://github.com/scroll-tech/go-ethereum/blob/scroll-v5.0.1/core/state_transition.go#L243
b. https://github.com/scroll-tech/go-ethereum/blob/scroll-v5.0.1/core/state_transition.go#L312
c. https://github.com/scroll-tech/go-ethereum/blob/scroll-v5.0.1/core/state_transition.go#L341
d. https://github.com/scroll-tech/go-ethereum/blob/scroll-v5.0.1/core/state_transition.go#L205
e. https://github.com/scroll-tech/go-ethereum/blob/scroll-v5.0.1/internal/ethapi/api.go#L1003
f. https://github.com/scroll-tech/go-ethereum/blob/scroll-v5.0.1/internal/ethapi/api.go#L1176
No need to check L1 data fee in
DoCall
, because it only checks if the tx can be successfully executed within a specified gas limit.2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.go
been updated?4. Breaking change label
Does this PR have the
breaking-change
label?