-
Notifications
You must be signed in to change notification settings - Fork 177
feat(benchmark): add SLOAD benchmark test with multi-contract support #2256
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?
feat(benchmark): add SLOAD benchmark test with multi-contract support #2256
Conversation
Add test_sload_empty_erc20_balanceof to benchmark SLOAD operations on non-existing storage slots using ERC20 balanceOf() queries. The idea of this benchmark is to exploit within a single or series of N contracts calls to non-existing addresses. On this way, we force clients to resolve as many tree branches as possible.
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.
Did a quick pass and it looks good to me overall.
I left a couple of questions as comments. Thanks!
pre: Alloc, | ||
fork: Fork, | ||
gas_benchmark_value: int, | ||
address_stubs, |
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.
I didn't quite expect the fixture to be directly used by the test tbh, it's an interesting workaround!
A couple things:
- It will produce unexpected behavior to someone running
execute
and not knowing the inner workings of this tests because it will try to use all the stubs, including those that are meant to be used by other tests. - The test will change its behavior depending on the stubs passed to the parameter in
execute
, which is not inherently a bad thing, but I think we should really think about it.
# 3. Most addresses have zero balance → empty storage slots | ||
# | ||
# WHY IT STRESSES CLIENTS: | ||
# - Each balanceOf() call forces a cold SLOAD on a likely-empty slot |
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.
How different would this be from a benchmark test that optimizes for SLOAD
s instead of trying to mimic the balanceOf()
behavior?
The things that come to mind are the extra jumps, keccak operations, and the fact that balanceOf()
(iirc) only does a single SLOAD
per subcall.
BloatNet SLOAD benchmark using ERC20 balanceOf queries on random addresses. | ||
|
||
This test: | ||
1. Auto-discovers ERC20 contracts from stubs (pattern: erc20_contract_*) |
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.
Ah, this answers my question on the first comment, I think it's ok then.
# RETURN costs 0 gas | ||
) | ||
|
||
num_contracts = len(address_stubs.root) |
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.
I think we have to do the pattern (erc20_contract_*
) discrimination here to have a proper number.
# In execute mode: stubs point to already-deployed contracts on chain | ||
# In fill mode: empty bytecode is deployed as placeholder | ||
erc20_addresses = [] | ||
for stub_name in address_stubs.root: |
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 loop also needs to discriminate using the pattern.
🗒️ Description
Add test_sload_empty_erc20_balanceof to benchmark SLOAD operations on non-existing storage slots using ERC20 balanceOf() queries.
The idea of this benchmark is to exploit within a single or series of N contracts calls to non-existing addresses. On this way, we force clients to resolve as many tree branches as possible.
✅ Checklist
tox
checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
type(scope):
.mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_from
marker.