-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(rpc): use gas_limit as gas_used if tx failed #18884
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
base: main
Are you sure you want to change the base?
Conversation
@mattsse please take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you elaborate on why this fixes the issue?
unclear to me why gas used needs to be swapped with gaslimit here
maybe @rakita knows
Hm maybe the expectation for gas returned is to return the max spent gas for the Halt, as halt spends all the gas. For Revert and Success it is not gas limit. |
@rakita shouldn't res.gas_used always have the actual gas used by the tx? so unclear why this is different here https://etherscan.io/tx/0x2a4b68d823b4468df290508f8466feaa588e8127b5680c0ebc99d22b4b0fed69 |
Sorry, this fix is incorrect, I think it's a bug in revm, @rakita could you please take a look at bluealloy/revm#3100 |
let gas_limit = tx_env.gas_limit(); | ||
let res = self.eth_api().inspect(db, evm_env, tx_env, &mut inspector)?; | ||
let gas_used = | ||
if res.result.is_success() { res.result.gas_used() } else { gas_limit }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be if res.result.is_halt() { gas_limit } else { res.result.gas_used() };
Revert version of return does not consume all the gas
@mattsse i think, the assumption is that Etherscan link that you shared shows last used gas, so maybe it is fine to leave as it is. Not sure if this It would be good to check how alloy/evm handles this, |
closes #18871
set gas_used=gas_limit for the failed tx