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

Kernel should not read target account of sys_call if gas is insufficient #194

Closed
frisitano opened this issue Apr 26, 2024 · 4 comments · Fixed by #199
Closed

Kernel should not read target account of sys_call if gas is insufficient #194

frisitano opened this issue Apr 26, 2024 · 4 comments · Fixed by #199

Comments

@frisitano
Copy link
Contributor

frisitano commented Apr 26, 2024

Whilst working on 0xPolygonZero/zero-bin#51 I have run into a number of cases in which the kernel panics. In these cases it appears that a call operation is invoked against some target account, however this call operation fails due to there being insufficient gas to start the call. As such, when I observe the output of debug_traceTransaction using both the callTracer and prestateTracer the call target account is not included. I believe this is because the call context is never actually created due to there being insufficient gas. As a consequence of this, when we create the block trace we do not include this account in the state trie to send to the prover. However, when I observe the logs from evm arithmetization I can see that it is trying to read the call target account from the state trie. It panics here because the account is not included in the trie for reasons described above.

I suspect that we need to add a check in the kernel context to evaluate if there is sufficient gas to enter into the call context before it is created. At this point this is somewhat of a "hunch" as I'm not an expert of the kernel. This problem may be relevant for other opcodes which also create child contexts, not just sys_call.

I have added specific instances of transactions of this nature to this comment and those following it:
0xPolygonZero/zero-bin#51 (comment)

cc: @Nashtare

@wborgeaud
Copy link
Contributor

We encountered a similar issue before and #124 fixed it for us. It ensures that we don't needlessly query state to compute the gas cost in call operations. Are you using the latest main? Otherwise, could you provide a tx where this issue occurs?

@frisitano
Copy link
Contributor Author

I branched from develop not main - this is the branch I'm working from #157.

Yes an example of this transaction is included in this comment here:

0xPolygonZero/zero-bin#51 (comment)

@frisitano
Copy link
Contributor Author

I wonder if it's because there is some value associated with the call and as such an additional gas cost which should be charged prior to the call operation. once again, just a hunch.

If call_value > 0 (sending value with call):
    base_gas += 9000

@wborgeaud
Copy link
Contributor

Yeah I think that's it. Currently we charge these 9000 gas after querying the state trie. In this tx for example, the call is made with 4323 gas left, which is enough for the cold access gas so we end up querying the state trie even though there's not enough gas to pay for the 9000 non-zero callvalue gas.
To fix this, we'd just have to charge the 9000 gas before querying the state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants