Enforce keyword-only arguments in calldata_gas_calculator methods #1714

akafle01 · 2025-06-04T23:01:27Z

🗒️ Description

This PR modifies the calldata_gas_calculator method in the base and derived fork classes to enforce keyword-only arguments for block_number and timestamp using *.

All method calls across the codebase have been updated to explicitly use keyword arguments.

I wasn't able to run the test suite locally due to a missing plugin error (pytest_plugins.concurrency), but the changes were made carefully and consistently. Please let me know if you'd like help resolving test setup.

🔗 Related Issues

Fixes #1709

✅ Checklist

All: Set appropriate labels for the changes.
All: Considered squashing commits to improve commit history.
All: Added an entry to CHANGELOG.md.
All: Considered updating the online docs in the ./docs/ directory.
Tests: All converted JSON/YML tests from ethereum/tests/tests/static have been assigned @ported_from marker.
Tests: A PR with removal of converted JSON/YML blockchain tests from ethereum/tests have been opened.
Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.

… calls

marioevz

Thanks for taking this.

My main concern is that this code could break if we convert only one of the fork methods at a time:

execution-spec-tests/src/ethereum_test_forks/transition_base_fork.py

Lines 64 to 81 in d6252aa

    
           def transition_method( 
        
               cls, 
        
               block_number: int = ALWAYS_TRANSITIONED_BLOCK_NUMBER, 
        
               timestamp: int = ALWAYS_TRANSITIONED_BLOCK_TIMESTAMP, 
        
           ): 
        
               kwargs = {} 
        
               if "block_number" in base_method_parameters: 
        
                   kwargs["block_number"] = block_number 
        
               if "timestamp" in base_method_parameters: 
        
                   kwargs["timestamp"] = timestamp 
        
               if getattr(base_method, "__prefer_transition_to_method__", False): 
        
                   return to_fork_method(**kwargs) 
        
               return ( 
        
                   to_fork_method(**kwargs) 
        
                   if block_number >= at_block and timestamp >= at_timestamp 
        
                   else from_fork_method(**kwargs) 
        
               )

I think we need to transform all the abstract method in BaseFork at the same time, which could be a major chore.

akafle01 added 2 commits June 4, 2025 18:56

Enforce keyword-only arguments in calldata_gas_calculator methods and…

9398d48

… calls

Fix line length violations reported by Ruff linter

81df804

marioevz reviewed Jun 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enforce keyword-only arguments in calldata_gas_calculator methods #1714

Enforce keyword-only arguments in calldata_gas_calculator methods #1714

Uh oh!

akafle01 commented Jun 4, 2025 •

edited

Loading

Uh oh!

marioevz left a comment

Uh oh!

Uh oh!

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

	def transition_method(
	cls,
	block_number: int = ALWAYS_TRANSITIONED_BLOCK_NUMBER,
	timestamp: int = ALWAYS_TRANSITIONED_BLOCK_TIMESTAMP,
	):
	kwargs = {}
	if "block_number" in base_method_parameters:
	kwargs["block_number"] = block_number
	if "timestamp" in base_method_parameters:
	kwargs["timestamp"] = timestamp

	if getattr(base_method, "__prefer_transition_to_method__", False):
	return to_fork_method(**kwargs)
	return (
	to_fork_method(**kwargs)
	if block_number >= at_block and timestamp >= at_timestamp
	else from_fork_method(**kwargs)
	)

Enforce keyword-only arguments in calldata_gas_calculator methods #1714

Are you sure you want to change the base?

Enforce keyword-only arguments in calldata_gas_calculator methods #1714

Uh oh!

Conversation

akafle01 commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

🔗 Related Issues

✅ Checklist

Uh oh!

marioevz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

akafle01 commented Jun 4, 2025 •

edited

Loading