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

Consider refactoring EIP 2930 access list lookups #200

Closed
frisitano opened this issue Apr 29, 2024 · 4 comments
Closed

Consider refactoring EIP 2930 access list lookups #200

frisitano opened this issue Apr 29, 2024 · 4 comments
Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate. performance Performance improvement related changes

Comments

@frisitano
Copy link
Contributor

frisitano commented Apr 29, 2024

Transactions can include EIP 2930 access lists containing address / storage key pairs which should be eagerly loaded into memory. In some cases these storage keys are never actually used by a transaction. As such both the kernel and the trace protocol are inefficient, and this potentially provides a DOS vector for the prover. We should consider refactoring this logic.

Quoted from @Nashtare:
Yeah agree, we could try revamping this part to remove the overhead, especially as SLOADs are a bit heavy and could be seen as some DOS entry door for the prover. Would need to think more about it how sound it would be to store arbitrary dummy value in non-accessed storage slots.

@4l0n50
Copy link
Contributor

4l0n50 commented Apr 30, 2024

If the addresses/storage keys are never accessed you still need to charge gas for them? At least EIP 2039 description says:

we charge additional gas for the access list: ACCESS_LIST_ADDRESS_COST gas per address and ACCESS_LIST_STORAGE_KEY_COST gas per storage key.

That would mean that you always need to iterate over the whole access list. On the other hand, gas cost would impose a limit on the size of the list.

As such both the kernel and the trace protocol are inefficient

At least the kernel only calls %insert_accessed_addresses_no_return (other than iterating over the list which is necessary for computing the gas), which takes a constant number of cycles (I don't remember, but it was something like 40 cycles)

@Nashtare
Copy link
Collaborator

That would mean that you always need to iterate over the whole access list.

We always go through it anyway to parse each addresses / storage keys.

On the other hand, gas cost would impose a limit on the size of the list.

Yeah, but I think it may still be prohibitive (30M gas limit allows for something like ~16k storage keys). If they are badly spread, the overhead would be quite large.

At least the kernel only calls %insert_accessed_addresses_no_return (other than iterating over the list which is necessary for computing the gas), which takes a constant number of cycles

When parsing the txn RLP and decoding the access list, we then call %insert_accessed_storage_keys_with_original_value for each storage key of the decoded address, which induces an SLOAD.

@4l0n50
Copy link
Contributor

4l0n50 commented May 2, 2024

we then call %insert_accessed_storage_keys_with_original_value for each storage key of the decoded address, which induces an SLOAD.

Ahh, I see. I was missing that part. I suppose the easiest solution would be to wait until we solve #172.

@Nashtare Nashtare added performance Performance improvement related changes crate: evm_arithmetization Anything related to the evm_arithmetization crate. labels May 5, 2024
@Nashtare Nashtare added this to the Performance Tuning milestone Jul 12, 2024
@0xPolygonZero 0xPolygonZero deleted a comment from GIgako19929 Sep 15, 2024
@Nashtare
Copy link
Collaborator

This isn't as much of a bottleneck anymore since #172. We can always revisit the approach once killer txns are weighted and gas repricing is done. Closing for now.

@github-project-automation github-project-automation bot moved this from Backlog to Done in Zero EVM Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate. performance Performance improvement related changes
Projects
Status: Done
Development

No branches or pull requests

3 participants