Skip to content

Conversation

felix314159
Copy link
Collaborator

@felix314159 felix314159 commented Sep 3, 2025

🗒️ Description

On ARM systems besu's evmtool might print the warning Unable to get SVE vector length on this system. Disabling SVE. Specify -XX:UseSVE=0 to shun this warning before the version when running evmtool --version. This PR improves the logic in ethereum_cli.py to handle this case. This PR contains a second fix, besu now shows its version as e.g. Hyperledger Besu evm 25.2.0 so the regex had to adjusted to allow words before the Besu part of the string.

Other t8n's behavior is unaffected by this PR (e.g. eels and geth's evm still work).

That being said, I'm unsure how useful besu t8n is for us currently. Commands like uv run fill --clean --evm-bin=$HOME/Documents/besu/besu-25.2.0/bin/evmtool tests/constantinople/eip1014_create2/test_recreate.py --eest-log-level=DEBUG -vv -s fail with it when they successfully fill with geth or eels. Maybe it has been broken ever since the spec-evm-resolver was introduced? Context #818

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast 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
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).
  • 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.
  • Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, just a single change.


if result.stderr:
# if there is an error try sth else, but don't treat besu's vector warning as error
if result.stderr and "SVE vector length" not in str(result.stderr):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's normally not the best approach to generalize something that only applies to a single item of a given set.

We can implement a new class method for this:

@classmethod
def process_stderr(cls, *, stderr: str) -> bool:
    """Process the stderr output and decide if the error is a breaking error for this specific tool."""
    # By default, any error is a breaking error.
    return True

Then overload this method in besu's implementation to do this extra conditional and return False if it's not a breaking error.

Copy link
Collaborator Author

@felix314159 felix314159 Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work. At that point in the logic you do not know whether the version you are getting is from Besu, Geth, ... so you would not be able to call a besu-specific function (the subclass check only comes later, when you are already passed this check). In order to not have to rewrite the entire from_binary_path logic I would prefer to keep things as is. But I agree that it's not a nice solution. But I doubt that we will ever get another execution client that (under certain conditions) prints a warning before giving the version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants