fix(execute): execute remote got broken

[felix314159]

🗒️ Description

The recent fix for execute recover broke execute remote due to double registering the chain-id flag. This PR not only fixes what got broken but also adds unit tests for discovering basic breakage like this (not even --help command worked so this can be detected now)

🔗 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 tox -e static

• 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.

Cute Animal Picture

[marioevz]

I would rather not having to patch like this. The places/plugins where we define --chain-id is currently visible in code + the pytest ini files, we could take some time to locate them and simply refactor based on that.

[felix314159]

u can locate them via rg -nP 'addoption\(\s*"--chain-id"' --multiline .

the problem we seem to have is that the logic for handling --chain-id and--rpc-chain-id deprecation stuff are kinda duplicated in remote.py (does not specify loading order) and execute.py (uses @pytest.hookimpl(tryfirst=True)). and we have multiple plugins with execute in their name which makes it hard to see which of them end up requiring chain-id or not.

if u want to do the clean solution (e.g. also get rid of --rpc-chain-id while we are at it, it has been described at 'deprecated' for a long time), we might as well move the parameter stuff into its own plugin (e.g. execute.flags) to have like a single source of truth. or if u don't like the current pr and want minimal changes we can use try .. except instead to get chain-id safely (only if it hasn't been gotten yet)

[marioevz]

Let's do it right and extract chain-id logic into its own plugin, but keep the deprecation notice (because nethermind might still be using it at the moment).

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.08%. Comparing base (705739a) to head (82dec61).
⚠️ Report is 1 commits behind head on forks/osaka.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##           forks/osaka    #1786   +/-   ##
============================================
  Coverage        86.08%   86.08%           
============================================
  Files              743      743           
  Lines            44072    44072           
  Branches          3891     3891           
============================================
  Hits             37938    37938           
  Misses            5656     5656           
  Partials           478      478           

Flag	Coverage Δ
unittests	86.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.