refactor(core): centralize EIP-7702 SetCode shape checks by 0xDevNinja · Pull Request #11641 · NethermindEth/nethermind

0xDevNinja · 2026-05-18T08:30:14Z

Changes

TransactionProcessor.ValidateStatic (Nethermind.Evm) and the NoContractCreationTxValidator / AuthorizationListTxValidator classes (Nethermind.Consensus) each carried their own copy of the EIP-7702 no-contract-creation and non-empty-authorization-list checks. The two sites cannot be collapsed into one — the consensus validators run for block validation / mempool admission, while the transaction-processor checks gate the RPC paths (eth_call, eth_estimateGas) that bypass TxValidator entirely — but they must not drift.
Extract the underlying predicates into Nethermind.Core.Validation.SetCodeTxValidation (ValidateNoContractCreation, ValidateAuthorizationList).
NoContractCreationTxValidator.IsWellFormed and AuthorizationListTxValidator.IsWellFormed are now one-liners delegating to the helper.
TransactionProcessor.ValidateStatic calls the same helpers, preserving its existing trace tags (SETCODE_TX_CREATE, EMPTY_AUTHORIZATION_LIST) and the MalformedTransaction.WithDetail(... (sender ...)) error shape.
New Nethermind.Core.Test.Validation.SetCodeTxValidationTests covers the helper directly.

No behavior change: error strings, trace tags, and consensus-side dispatch via the TxValidator register table are unchanged. The PR is a pure deduplication so that future EIP-7702 rule edits land in one place rather than two.

Types of changes

What types of changes does your code introduce?

Bugfix (a non-breaking change that fixes an issue)
New feature (a non-breaking change that adds functionality)
Breaking change (a change that causes existing functionality not to work as expected)
Optimization
Refactoring
Documentation update
Build-related changes
Other: Description

Testing

Requires testing

Yes
No

If yes, did you write tests?

Yes
No

Notes on testing

New SetCodeTxValidationTests — 5/5 in Nethermind.Core.Test.
TxValidatorTests + TransactionProcessorEip7702Tests — 164/164 in Nethermind.Blockchain.Test.
EthRpcModuleTests (covers eth_call and eth_estimateGas SetCode rejection paths via TransactionProcessor) — 381/383 in Nethermind.JsonRpc.Test (2 unrelated skipped).
dotnet format whitespace --folder clean.

Documentation

Requires documentation update

Yes
No

Requires explanation in Release Notes

Yes
No

LukaszRozmej · 2026-05-18T11:03:59Z

@claude review

claude · 2026-05-18T11:04:17Z

Claude finished @LukaszRozmej's task in 3m 43s —— View job

PR Review: refactor(core): centralize EIP-7702 SetCode shape checks

Read changed files
Review SetCodeTxValidation.cs (new helper)
Review SetCodeTxValidationTests.cs (new tests)
Review TxValidator.cs changes
Review TransactionProcessor.cs changes
Check rules files for C# conventions
Post findings

Summary

Clean, well-scoped refactoring. Behavior is fully preserved — I verified the before/after logic in both sites:

tx.IsContractCreation → SetCodeTxValidation.ValidateNoContractCreation(tx) ✓ identical predicate
!tx.HasAuthorizationList → !SetCodeTxValidation.ValidateAuthorizationList(tx) ✓ identical predicate (the Type == TxType.SetCode leg of HasAuthorizationList is already guaranteed by the SupportsAuthorizationList guard)
Error strings, trace tags (SETCODE_TX_CREATE, EMPTY_AUTHORIZATION_LIST), and MalformedTransaction.WithDetail("… (sender …)") shape are all unchanged

Architecture, test coverage, XML docs, and the class-level <remarks> explaining the two-site constraint are all solid.

3 Low findings — none block merge.

Low

1. SetCodeTxValidationTests.cs:71 — new[] syntax instead of C# 14 collection expression

// current
.WithAuthorizationCode(new[] { AuthorizationTuple() })

// preferred (C# 14, per project "Prefer the latest C# syntax and conventions")
.WithAuthorizationCode([AuthorizationTuple()])

2. SetCodeTxValidationTests.cs:17 — helper method name shadows the type

private static AuthorizationTuple AuthorizationTuple() => ...

The method is named identically to the AuthorizationTuple type it returns, which can confuse readers and compilers resolving overloads. Prefer SampleAuthorizationTuple() or CreateAuthorizationTuple().

3. SetCodeTxValidationTests.cs:42–64 — two tests with the same body, different inputs (AGENTS.md guideline)

ValidateAuthorizationList_returns_error_when_list_is_null and ValidateAuthorizationList_returns_error_when_list_is_empty differ only in how the transaction is built. Per the project's AGENTS.md: "When tests differ only by inputs and expected outputs, parameterize a single test with [TestCase] or [TestCaseSource] rather than copy-pasting the body." Since [TestCase] can't carry builder lambdas, a [TestCaseSource]-backed pair of transactions would clean this up, though for just 2 cases the two-method form is tolerable.

Notes

The 4-line explanatory comment in TransactionProcessor.cs (lines 614–617) is appropriate — it captures an architectural invariant (two sites, one truth) that is not derivable from the code alone, exactly the use case the project guidelines describe for inline comments.
NoContractCreationTxValidator and AuthorizationListTxValidator correctly implement only the 2-arg ITxValidator.IsWellFormed overload; the interface's default implementation forwards the 3-arg call through, so nothing is missed.

Verdict: Mergeable. The Low findings are advisory; none affect correctness or behavior.

claude · 2026-05-18T11:07:58Z

+    {
+        Transaction tx = Build.A.Transaction
+            .To(TestItem.AddressA)
+            .WithAuthorizationCode(new[] { AuthorizationTuple() })


Prefer C# 14 collection expression over new[]:

Suggested change

.WithAuthorizationCode(new[] { AuthorizationTuple() })

.WithAuthorizationCode([AuthorizationTuple()])

claude · 2026-05-18T11:08:00Z

+[Parallelizable(ParallelScope.All)]
+public class SetCodeTxValidationTests
+{
+    private static AuthorizationTuple AuthorizationTuple() =>


The method name AuthorizationTuple() exactly shadows the AuthorizationTuple type it returns. Consider SampleAuthorizationTuple() or CreateAuthorizationTuple() to avoid ambiguity for future readers.

claude · 2026-05-18T11:08:02Z

+    public void ValidateAuthorizationList_returns_error_when_list_is_null()
+    {
+        Transaction tx = Build.A.Transaction.To(TestItem.AddressA).TestObject;
+
+        ValidationResult result = SetCodeTxValidation.ValidateAuthorizationList(tx);
+
+        result.AsBool().Should().BeFalse();
+        result.Error.Should().Be(TxErrorMessages.MissingAuthorizationList);
+    }
+
+    [Test]
+    public void ValidateAuthorizationList_returns_error_when_list_is_empty()
+    {
+        Transaction tx = Build.A.Transaction
+            .To(TestItem.AddressA)
+            .WithAuthorizationCode(Array.Empty<AuthorizationTuple>())
+            .TestObject;
+
+        ValidationResult result = SetCodeTxValidation.ValidateAuthorizationList(tx);
+
+        result.AsBool().Should().BeFalse();
+        result.Error.Should().Be(TxErrorMessages.MissingAuthorizationList);
+    }


These two tests differ only by the auth-list value supplied (null vs empty array). Per AGENTS.md, consider collapsing them into a single [TestCaseSource]-backed test. For just two cases the two-method form is tolerable, but flagging per project guidelines.

Address review feedback on NethermindEth#11641: - Rename `AuthorizationTuple()` helper to `CreateAuthorizationTuple()` so it no longer shadows the `AuthorizationTuple` type it returns. - Replace `new[] { ... }` array literal with C# 14 collection expression `[...]`. - Collapse the two `_returns_error_when_list_is_null` and `_returns_error_when_list_is_empty` tests into one `[TestCaseSource]`-driven method (`ValidateAuthorizationList_returns_error_when_list_is_missing`), per AGENTS.md test-parameterization guideline.

flcl42 · 2026-05-18T16:04:27Z

            if (tx.SupportsAuthorizationList)
            {
-                if (tx.IsContractCreation)
+                // SetCode-specific shape checks live in Nethermind.Core.Validation.SetCodeTxValidation


No need in such comment

@flcl42

Per @flcl42 review on NethermindEth#11641 — the comment block above the SetCodeTxValidation calls in TransactionProcessor.ValidateStatic restated context that lives in the helper class XML docs.

`TransactionProcessor.ValidateStatic` (Evm) and the `NoContractCreationTxValidator` / `AuthorizationListTxValidator` classes (Consensus) each carried their own copy of the EIP-7702 no-contract-creation and non-empty-authorization-list checks. The two sites cannot be collapsed into one — the consensus validators run for block validation / mempool admission, while the transaction-processor checks gate the RPC paths (eth_call, eth_estimateGas) that bypass `TxValidator` entirely — but they must not drift. Extract the underlying predicates into a new `Nethermind.Core.Validation.SetCodeTxValidation` static helper (`ValidateNoContractCreation`, `ValidateAuthorizationList`) and have both sites delegate to it: - `NoContractCreationTxValidator.IsWellFormed` and `AuthorizationListTxValidator.IsWellFormed` are now one-liners that call into the helper. - `TransactionProcessor.ValidateStatic` calls the same helpers, preserving its existing trace tags (`SETCODE_TX_CREATE`, `EMPTY_AUTHORIZATION_LIST`) and the `MalformedTransaction.WithDetail(... (sender ...))` error shape. No behavior change: error strings, trace tags, and consensus-side dispatch via `TxValidator` register table are unchanged. `Nethermind.Core.Test.Validation.SetCodeTxValidationTests` covers the helper directly. Existing `TxValidatorTests`, `TransactionProcessorEip7702Tests`, and the EthRpcModule `eth_call` / `eth_estimateGas` tests still pass and continue to exercise both call paths end-to-end. Closes NethermindEth#11619

Address review feedback on NethermindEth#11641: - Rename `AuthorizationTuple()` helper to `CreateAuthorizationTuple()` so it no longer shadows the `AuthorizationTuple` type it returns. - Replace `new[] { ... }` array literal with C# 14 collection expression `[...]`. - Collapse the two `_returns_error_when_list_is_null` and `_returns_error_when_list_is_empty` tests into one `[TestCaseSource]`-driven method (`ValidateAuthorizationList_returns_error_when_list_is_missing`), per AGENTS.md test-parameterization guideline.

@flcl42

Per @flcl42 review on NethermindEth#11641 — the comment block above the SetCodeTxValidation calls in TransactionProcessor.ValidateStatic restated context that lives in the helper class XML docs.

LukaszRozmej · 2026-05-25T09:10:58Z

@claude re-review

claude · 2026-05-25T09:11:14Z

Claude finished @LukaszRozmej's task in 1m 28s —— View job

Re-review: refactor(core): centralize EIP-7702 SetCode shape checks

Read prior review findings and comments
Read current state of changed files
Check if previous findings were addressed
Look for any new issues
Post updated review

Summary

All three Low findings from the first review have been resolved, and flcl42's comment was addressed:

Prior finding	Status
`new[]` → collection expression	✓ Fixed (`[CreateAuthorizationTuple()]` at line 71)
`AuthorizationTuple()` shadows type name	✓ Fixed → renamed to `CreateAuthorizationTuple()`
Duplicate null/empty test bodies	✓ Fixed → collapsed into `[TestCaseSource(nameof(MissingAuthorizationListCases))]`
Explanatory comment in `TransactionProcessor.cs` (flcl42)	✓ Dropped in `d3c0175`

Remaining nit (non-blocking)

SetCodeTxValidationTests.cs:50 — Array.Empty<AuthorizationTuple>() is now inside a [TestCaseSource] factory. Since IDE0028 is enforced in linting (commit f56cf7f on master), this could be [] instead, but it's in a test factory body where type inference is clear from the method signature. Not a blocker.

Verdict: Mergeable. The PR is a clean, well-scoped deduplication with no behavior change. Code, tests, and documentation are all in good shape.

0xDevNinja requested review from LukaszRozmej, MarekM25, benaadams and flcl42 as code owners May 18, 2026 08:30

github-actions Bot added eip evm refactoring labels May 18, 2026

claude Bot reviewed May 18, 2026

View reviewed changes

LukaszRozmej approved these changes May 18, 2026

View reviewed changes

flcl42 reviewed May 18, 2026

View reviewed changes

0xDevNinja added 3 commits May 25, 2026 11:47

refactor(evm): drop redundant SetCode-validation comment

d3c0175

Per @flcl42 review on NethermindEth#11641 — the comment block above the SetCodeTxValidation calls in TransactionProcessor.ValidateStatic restated context that lives in the helper class XML docs.

0xDevNinja force-pushed the fix/11619-centralize-setcode-tx-validation branch from 02ef34e to d3c0175 Compare May 25, 2026 06:20

LukaszRozmej approved these changes May 25, 2026

View reviewed changes

benaadams approved these changes May 26, 2026

View reviewed changes

LukaszRozmej merged commit f25efeb into NethermindEth:master May 26, 2026
544 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(core): centralize EIP-7702 SetCode shape checks#11641

refactor(core): centralize EIP-7702 SetCode shape checks#11641
LukaszRozmej merged 3 commits into
NethermindEth:masterfrom
0xDevNinja:fix/11619-centralize-setcode-tx-validation

0xDevNinja commented May 18, 2026

Uh oh!

LukaszRozmej commented May 18, 2026

Uh oh!

claude Bot commented May 18, 2026 •

edited

Loading

Uh oh!

claude Bot May 18, 2026

Uh oh!

claude Bot May 18, 2026

Uh oh!

claude Bot May 18, 2026

Uh oh!

flcl42 May 18, 2026

Uh oh!

LukaszRozmej commented May 25, 2026

Uh oh!

claude Bot commented May 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	.WithAuthorizationCode(new[] { AuthorizationTuple() })
	.WithAuthorizationCode([AuthorizationTuple()])

Conversation

0xDevNinja commented May 18, 2026

Changes

Types of changes

What types of changes does your code introduce?

Testing

Requires testing

If yes, did you write tests?

Notes on testing

Documentation

Requires documentation update

Requires explanation in Release Notes

Uh oh!

LukaszRozmej commented May 18, 2026

Uh oh!

claude Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: refactor(core): centralize EIP-7702 SetCode shape checks

Summary

Low

Notes

Uh oh!

claude Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

flcl42 May 18, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej commented May 25, 2026

Uh oh!

claude Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-review: refactor(core): centralize EIP-7702 SetCode shape checks

Summary

Remaining nit (non-blocking)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

claude Bot commented May 18, 2026 •

edited

Loading

claude Bot commented May 25, 2026 •

edited

Loading