-
Notifications
You must be signed in to change notification settings - Fork 414
fix(test-benchmark): support blobhash benchmark
#2067
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(test-benchmark): support blobhash benchmark
#2067
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2067 +/- ##
================================================
Coverage 86.14% 86.14%
================================================
Files 599 599
Lines 39472 39472
Branches 3780 3780
================================================
Hits 34002 34002
Misses 4848 4848
Partials 622 622
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Can we add a pytest skip for execute here for now? With a todo @pytest.mark.execute(pytest.mark.skip(reason="Blob txs not supported by execute")) |
spencer-tb
left a comment
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.
BLOBHASH reads from the current tx blob array, not a previous tx, so we can't go the setup_tx route. I added an updated test implementation below.
For the --fixed-opcode-count mode:
- Prague: No tx gas limit cap. Single tx with effectively unlimited gas (set to 1T).
Op.POPkeeps the stack clean, enabling unlimited iterations. - Osaka+: Has a 16M tx gas limit cap. Single tx capped at 16M gas, no splitting. This only lets us get to a limit of 1200 (x1000) for the count.
For the --gas-benchmark-values mode:
- Prague: No tx gas limit cap, so no splitting occurs. Single blob tx works fine.
- Osaka+: Has a 16M tx gas limit cap which causes transaction splitting. Each split tx would be a blob tx, and multiple blob txs per block could exceed the block blob limit.
The test below calculates required_splits = gas_benchmark_value / tx_gas_limit_cap and compares against max_blobs_per_block. If splits exceed the blob limit, the test is skipped for that fork. If we want to hit 150M we need 10 blobs, 9 will only get us to 144M. This means we should probably fill for >BPO1, maybe Amsterdam. If Amsterdam inherits BPO2 then we can reach around 336M.
@pytest.mark.repricing
@pytest.mark.parametrize(
"blob_present",
[
pytest.param(0, id="no_blobs"),
pytest.param(1, id="one_blob"),
],
)
def test_blobhash(
fork: Fork,
benchmark_test: BenchmarkTestFiller,
pre: Alloc,
blob_present: int,
fixed_opcode_count: int | None,
gas_benchmark_value: int,
) -> None:
"""Benchmark BLOBHASH instruction."""
tx_kwargs: dict = {}
if blob_present:
cap = fork.transaction_gas_limit_cap()
if fixed_opcode_count is None and cap is not None:
# Check if blob tx splits would exceed block blob limit
required_splits = math.ceil(gas_benchmark_value / cap)
max_blobs = fork.max_blobs_per_block()
if required_splits > max_blobs:
pytest.skip(
f"Blob tx needs {required_splits} splits but fork allows "
f"{max_blobs} blobs/block"
)
tx_kwargs = {
"ty": TransactionType.BLOB_TRANSACTION,
"max_fee_per_blob_gas": fork.min_base_fee_per_blob_gas(),
"blob_versioned_hashes": add_kzg_version(
[i.to_bytes(32, "big") for i in range(blob_present)],
BlobsSpec.BLOB_COMMITMENT_VERSION_KZG,
),
}
benchmark_test(
target_opcode=Op.BLOBHASH,
code_generator=ExtCallGenerator(
attack_block=Op.POP(Op.BLOBHASH(0)),
tx_kwargs=tx_kwargs,
),
)8c31bb3 to
5976565
Compare
|
@spencer-tb Thanks a lot!! One small thing: we could remove |
spencer-tb
left a comment
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.
<3
🗒️ Description
The original blobhash benchmark is failing in every mode, this PR supports it under
fillmode with--gas-benchmark-valuesand--fixed-opcode-countflag. But it still needs some update to supportexecute remote, since now it does not support blob transactions.🔗 Related Issues or PRs
PR #1695
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture