Skip to content

fix(test-rpc): handle extra fields in TransactionByHashResponse after CamelModel extra="forbid"#1997

Closed
LouisTsai-Csie wants to merge 1 commit intoethereum:forks/amsterdamfrom
LouisTsai-Csie:fix-pydantic-rule
Closed

fix(test-rpc): handle extra fields in TransactionByHashResponse after CamelModel extra="forbid"#1997
LouisTsai-Csie wants to merge 1 commit intoethereum:forks/amsterdamfrom
LouisTsai-Csie:fix-pydantic-rule

Conversation

@LouisTsai-Csie
Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie commented Jan 9, 2026

🗒️ Description

Fixes TransactionByHashResponse to work correctly after #1989 added extra="forbid" to CamelModel

After #1989, CamelModel now rejects extra fields. This broke TransactionByHashResponse when parsing RPC responses from execution clients because:

  1. Missing transactionIndex field: Standard field returned by all clients per the Ethereum JSON-RPC spec
  2. Duplicate yParity/v fields: Modern clients return both for EIP-1559+ transactions
  3. Parent validator strips hash: Transaction.strip_hash_from_t8n_output removes the hash field before TransactionByHashResponse can use it

Solution

  • Add transaction_index field to match the standard RPC response
  • Use mode="wrap" validator to run BEFORE parent's validators, preserving hash under _preserved_hash
  • Strip duplicate yParity when both v and yParity are present
  • Use validation_alias=AliasChoices("hash", "_preserved_hash") as fallback for the hash field

Verified with:

# create anvil node first
anvil --accounts 1 --balance 30000000

# Run the benchmark
execute remote --rpc-endpoint=http://127.0.0.1:8545 --fork Prague -m stateful tests/benchmark/stateful/bloatnet/test_multi_opcode.py::test_bloatnet_balance_extcodecopy -s

🔗 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

Put a link to a cute animal picture inside the parenthesis-->

@LouisTsai-Csie LouisTsai-Csie self-assigned this Jan 9, 2026
@LouisTsai-Csie LouisTsai-Csie added A-test-rpc Area: execution_testing.rpc C-refactor Category: refactor labels Jan 9, 2026
@codecov
Copy link

codecov bot commented Jan 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.33%. Comparing base (8c9e889) to head (fd88214).
⚠️ Report is 3 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #1997   +/-   ##
================================================
  Coverage            86.33%   86.33%           
================================================
  Files                  538      538           
  Lines                34557    34557           
  Branches              3222     3222           
================================================
  Hits                 29835    29835           
  Misses                4148     4148           
  Partials               574      574           
Flag Coverage Δ
unittests 86.33% <ø> (ø)

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.

@fselmo
Copy link
Contributor

fselmo commented Jan 9, 2026

Hey @LouisTsai-Csie. As we discussed, I put up a separate fix in #2000. I think the move here is to loosen restrictions for internal models on extra fields, especially things like RPC classes (TransactionByHashResponse). I think if we ever build an RPC simulator for compat tests, this would really come in handy... but for most calls I think we just want to pass through whatever the client returns.

I put up a PR that fixes #1998 as well as #1999 (I created an issue to track this). Let me know what you think.

@fselmo
Copy link
Contributor

fselmo commented Jan 9, 2026

closing in favor of #2000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-test-rpc Area: execution_testing.rpc C-refactor Category: refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants