Skip to content

Restore legacy-tx decoding in cs-test - #4

Merged
llbartekll merged 2 commits into
mainfrom
restore-legacy-tx-decoder
May 27, 2026
Merged

llbartekll merged 2 commits into
mainfrom
restore-legacy-tx-decoder

Conversation

@llbartekll

Copy link
Copy Markdown
Owner

Summary

Reapplies @manuelwedler's c19a9e6 (Support legacy transactions in cs-test rawTx decoder), which we reverted on the original review of #1.

Why

The revert rationale at the time was "no current registry fixture exercises legacy txs" — true for the single aave example fixture in registry PR #2586 then, wrong for the bulk migration Manuel is now landing in manuelwedler/clear-signing-erc7730-registry#2.

That migration ports dozens of historical fixtures, many of which carry pre-EIP-1559 rawTx values starting with RLP list headers (0xc0+) — e.g. 1inch AggregationRouterV3 cases beginning 0xf985d1…. Without legacy decoding, our runner reports error: decode rawTx: legacy transactions are not supported for every such case, breaking his column.

Re-evaluating the original critique

  • EIP-155 unsigned-vs-signed disambiguation via empty r/s: correct. Matches how Geth / ethers encode the two forms.
  • Pre-EIP-155 mainnet fallback: conventional. Pre-EIP-155 txs don't encode chainId; defaulting to 1 is what every parser does.
  • Unused _gas_price binding: cosmetic, not a behavior issue.

So the original commit was fine; the revert was the mistake.

Test plan

  • cargo test -p cs-test — 19 lib + 4 smoke passing
  • cargo clippy -p cs-test --all-targets -- -D warnings — clean
  • End-to-end against Manuel's migrated registry/1inch/testsv2/calldata-AggregationRouterV3.tests.json: both cases now produce pass/fail (one of each, as expected) instead of decode errors.

Co-author attribution preserved on the reapply commit.

Reapplies c19a9e6e29aaf09da85cbaf6d8258d41d52dc6ee5, which was reverted
in bfc2d44 on the original review of #1.

The revert rationale ("no current registry fixture exercises legacy
txs") was based on the single example fixture in registry PR #2586 at
the time. Manuel's bulk migration in manuelwedler/clear-signing-erc7730-registry#2
now ports dozens of real historical fixtures, many of which carry
legacy (pre-EIP-1559) rawTx — e.g. 1inch AggregationRouterV3 cases
starting with `0xf9...` RLP list headers. Without legacy decoding our
runner reports `error: decode rawTx: legacy transactions are not
supported` for every such case, breaking his migration.

Re-reading the original commit:
- The EIP-155 unsigned-vs-signed disambiguation via empty r/s byte
  strings is the correct mechanism (matches how Geth and ethers encode
  the two forms).
- The pre-EIP-155 mainnet fallback is conventional — pre-EIP-155 txs
  don't encode chainId, so callers default to 1 (Geth does the same).
- The `_gas_price` unused binding is cosmetic, not a behavior issue.

Verified end-to-end against the migrated 1inch AggregationRouterV3
fixture: both cases now produce pass/fail results instead of decode
errors.

Co-Authored-By: Manuel Wedler <34456797+manuelwedler@users.noreply.github.com>
`cargo fmt --check` flagged a single-line anyhow! call in the reapplied
legacy path. Wrap to multi-line to match the normalization that landed
on cs-test in the merged work.
@llbartekll
llbartekll merged commit f0c57fc into main May 27, 2026
2 of 3 checks passed
llbartekll added a commit that referenced this pull request Jun 4, 2026
Resolve library-side output mismatches surfaced by the upstream registry's
v2 test migration (manuelwedler/clear-signing-erc7730-registry #2/#3/#4):

- date: RFC-3339 "Z" suffix instead of " UTC"
- token amounts: zero renders "0" (not "0.0")
- addresses: EIP-55 checksum on the EIP-712 and raw-format paths (calldata parity)
- array-of-struct scopes: element-major ordering (fixes wrong field labels)
- interpolatedIntent: match {name} templates against #.-prefixed field paths
- amount format: render as a native-currency amount
- unit base: resolve $.metadata.constants.* references

Add cs-test regression fixtures (degate, lido, uniswap, aave, yieldxyz)
vendored from the registry. The nested-call case (kiln) is #[ignore]'d:
the runner resolves only the outer descriptor, a harness gap rather than an
engine bug. The aave Borrow case is dropped (its @.from field is unavailable
to the runner).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant