-
Notifications
You must be signed in to change notification settings - Fork 325
chore: remove hardcoded fork reference osaka and fetch latest fork #1337
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: forks/osaka
Are you sure you want to change the base?
chore: remove hardcoded fork reference osaka and fetch latest fork #1337
Conversation
* Update to latest EIP-7918 * EIP-7918 t8n tool update --------- Co-authored-by: Sam Wilson <[email protected]> Co-authored-by: Guruprasad Kamath <[email protected]>
* feat(vm): add eip-7939 count leading zeros opcode. * Update src/ethereum/osaka/vm/instructions/bitwise.py Co-authored-by: Guruprasad Kamath <[email protected]> * code formatting --------- Co-authored-by: Guruprasad Kamath <[email protected]> Co-authored-by: Guruprasad Kamath <[email protected]>
…state management and interpreter. Updated comments for clarity on the implications of SELFDESTRUCT.
…MAX_CODE_SIZE across multiple modules
update exceptions for other forks fix tests use fork module rename to NonceOverFlowError revert load test catching specific exc style: indent comment line
- ignore all pytest functions - remove unused variables, functions, methods, and classes - create vulture whitelist and mark some code that either implicitly gets called or only gets called during test runs for vulture to ignore
Signed-off-by: bytetigers <[email protected]>
* update dependencies * add fill to the entry points * add eest tests * run eest tests in py3 * create EELS TransitionTool subclass * post review fix and latest eest * remove pytest-eest.ini * minor fix: fix tox config * minor fix: add vulture exception * run eest tests in pypy * update optimized tox command
pyproject.toml
Outdated
@@ -34,6 +34,7 @@ dependencies = [ | |||
package-dir = {"" = "src"} | |||
packages = [ | |||
"ethereum_spec_tools", | |||
"ethereum_spec_tools.cli", |
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.
The other CLI tools exist directly under ethereum_spec_tools
. For consistency, this tool should as well.
src/ethereum_spec_tools/cli/forks.py
Outdated
from hardfork import Hardfork | ||
|
||
|
||
def get_fork_by_index(forks: List[Hardfork], index: int) -> Optional[Hardfork]: |
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'm partial to omitting get_
.
def get_fork_by_index(forks: List[Hardfork], index: int) -> Optional[Hardfork]: | |
def fork_by_index(forks: List[Hardfork], index: int) -> Optional[Hardfork]: |
src/ethereum_spec_tools/cli/forks.py
Outdated
if index < 0: | ||
# Negative indexing: -1 is last, -2 is second last, etc. | ||
if abs(index) <= len(forks): | ||
return forks[index] | ||
else: | ||
return None | ||
else: | ||
# Positive indexing: 0 is first (Frontier), 1 is second, etc. | ||
if index < len(forks): | ||
return forks[index] | ||
else: | ||
return None |
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.
Python lists natively support negative indexing. Is there something that makes return forks[index]
infeasible?
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.
yes if the index is out if bounds, it looks more verbose than a try-catch
block which is slower in case an exception occurs
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 don't think we care that much about performance in this tool 🤣
commands = | ||
fill \ | ||
-m "not slow and not zkevm and not benchmark" \ | ||
-n auto --maxprocesses 6 \ | ||
--basetemp="{temp_dir}/pytest" \ | ||
--clean \ | ||
eest_tests/execution-spec-tests/tests | ||
fill \ | ||
bash -c 'fill \ |
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'll break our CI on Windows... There's no other way to do a variable substitution from a command?
If not, I don't think this approach will work after all. We might have to do it as a pytest plugin (or something in conftest.py
) that takes a --fork-number
argument instead of a separate command line tool.
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.
We can create platform specific commands separately for windows, although I'm not sure if that will work
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 would like to avoid anything platform specific if we can.
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 couldn't think of any other way to implement variable substitution that is platform agnostic. The only way that comes to my mind is probably running it as a python script
@SamWilsn I've added all the changes you've mentioned, please do take a look |
301eb89
to
5a49b2f
Compare
I've rebased git checkout forks/osaka
git pull
git checkout tox/remove-hardcoded-fork-osaka
git reset --hard forks/osaka
git cherry-pick 6e993fca90bea968771bf6decebb2cc5e6cfed4d
git cherry-pick 52403548c40109a32b66316224ebb6141c580ea1 |
For some reason, it's showing my |
What was wrong?
Related to Issue #1330
Hardcoded fork reference osaka was being used in
tox.ini
How was it fixed?
Implemented a CLI command
ethereum-spec-forks
to fetch hardfork information. Used it intox.ini
to fetch the latest fork.Closes #1330
Cute Animal Picture