refactor(evm): drop the unused EIP-8272 native recent-root write path

[AnkushinDaniil]

Changes

• Remove RecentRootStore.Write and its private helper SourceId, plus the test-only convenience overload they served.

Recent-root storage (EIP-8272) is written by the RECENT_ROOT_ADDRESS predeploy bytecode during ordinary execution, not by the client. RecentRootStore.Write was a leftover native-write path from before the predeploy approach and had no production caller: the only references were in its own unit tests. Block processing installs the predeploy code (PredeployInstaller) and the SSTORE is performed by the EVM executing that code, so there is no C# write hook and there should not be one. SourceId was reachable only from Write, so it goes with it.

The read path (EntryHash, StorageKey, IsReferenceValid) stays: it is called by the reference-validity check on the execution path. The SourceId/Write derivations move into the test file as documented private helpers, so the read-path tests still have committed entries to validate against.

Types of changes

• Refactoring (removing dead code)

Testing

• Yes

RecentRootStoreTests rewritten against the read path; dotnet test Nethermind.Evm.Test --filter RecentRootStoreTests passes 7/7.

[claude]

Claude finished @AnkushinDaniil's task in 4m 57s —— View job

---

Review: refactor(evm): drop the unused EIP-8272 native recent-root write path

• Gather context (diff, surrounding files)
• Verify Write/SourceId have no production callers
• Check read-path & test-helper equivalence
• Build + run tests — not possible (dotnet build/dotnet test are outside this job's allowed tools; see note below)
• Post review

---

Verdict

The code change is safe and correct. My one substantive concern is with the justification, not the deletion: two load-bearing claims in the PR description don't hold on this branch.

Findings: 0 Critical · 0 High · 1 Medium · 1 Low

---

Medium — the stated rationale is not true at this commit (inline)

The description says the read path "is called by the reference-validity check on the execution path" and that "Block processing installs the predeploy code (PredeployInstaller)". Verified against the branch:

Claim	Reality
IsReferenceValid called on the execution path	0 callers outside RecentRootStoreTests
PredeployInstaller installs the predeploy	No such type exists in src/Nethermind
Read path is live	Nothing outside RecentRootStore.cs references Eip8272Constants.RecentRootAddress; MaxRecentRootReferences has no references at all

So RecentRootStore is entirely unreachable from production today — this removes one dead method from a dead class, not the dead path from a live one. That's a fine thing to do if EIP-8272 is landing incrementally (which Eip8272Constants' "Provisional: spec address is TBD" suggests), but the asymmetry between "drop the write path" and "keep the read path" should be justified by "groundwork for the follow-up that wires frame-transaction validation" rather than by a caller that doesn't exist. If no follow-up is imminent, AGENTS.md ("prefer the solution that removes code"; "If a change makes existing code unused, remove it") argues for dropping RecentRootStore + Eip8272Constants wholesale.

The new <remarks> on RecentRootStore has the same issue: accurate as a description of the EIP, but it reads as though Nethermind installs and executes that predeploy today.

Low — dropped consensus-relevant comment + magic numbers (inline)

The removed SourceId carried // Consensus-critical, spec-ambiguous: 32-byte-padded address matches the only existing implementation (spec text says 20 bytes). The test-file replacement states the 32-byte padding as settled fact. That was the only record in the repo of Nethermind choosing the reference implementation over the spec text, and it stays relevant because source_id is the input to IsReferenceValid — whoever wires EIP-8272 up will have to re-derive it. A suggestion that restores the note (and swaps the three literal 32s for Hash256.Size, per .agents/rules/coding-style.md) is in the inline comment.

---

What checks out

• Test-helper equivalence is exact. The new Write helper (RecentRootStoreTests.cs:145-152) reproduces the deleted production body byte-for-byte: same ring index (slot % RecentRootLength), same EntryHash(sourceId, slot, root), same WithoutLeadingZeros() minimal-big-endian encoding that IsReferenceValid re-pads at RecentRootStore.cs:55-57. No assertion changes meaning.
• Building the cell through the public StorageKey rather than the private RingBufferCell is an improvement — the test now exercises the same derivation the read path uses instead of trusting a shared private helper.
• No coverage loss. SourceId_is_deterministic_and_distinct_per_input tested an API that no longer exists; correct to delete. The ring-buffer aliasing, window-boundary, and mismatch tests all survive.
• RingBufferCell is still live (used by IsReferenceValid:47), so it correctly stays.
• The deleted Write ignored its IReleaseSpec spec parameter entirely — it never fork-gated, so dropping it loses nothing.
• No API-compat risk — RecentRootStore is provisional, in-repo-only, EIP-draft code.
• Compile check (static): the new using Nethermind.Core.Extensions covers WithoutLeadingZeros; ValueHash256.ToUInt256() (Hash256.cs:105), StorageCell (Nethermind.Core), and UInt256.Zero are all still reachable from the remaining usings. Removing Nethermind.Core.Specs / Nethermind.Specs.Forks is correct — the last uses went with Spec.
• No performance or security surface: hot paths untouched, stackalloc sizes unchanged.

⚠️ I could not run dotnet build or dotnet test — both require approval beyond this job's --allowedTools. The compile analysis above is static only; the author reports RecentRootStoreTests passing 7/7 locally, and the count matches the 7 [Test] methods in the file.
· daniil/drop-recent-root-native-write

[claude]

Low — the consensus-relevant part of the old comment was dropped.

The production SourceId carried:

// Consensus-critical, spec-ambiguous: 32-byte-padded address matches the only existing implementation (spec text says 20 bytes).

The replacement comment now states the 32-byte padding as if it were settled. That divergence note is the only place in the repo recording that Nethermind followed the reference implementation over the spec text — and whoever wires up EIP-8272 will have to re-derive source_id (it's the input to IsReferenceValid), so the note is still load-bearing even though the derivation now lives in a test.

Also, the helper reintroduces the literal 32 three times where the production code used named constants (.agents/rules/coding-style.md: "Prefer constants to magic numbers where the value is reused").

Suggested change

	// The predeploy derives source_id as keccak256(msg.sender-left-padded-to-32 || salt); mirror it here
	// to build test inputs for the read-path assertions above.
	private static ValueHash256 SourceId(Address sourceAddress, in ValueHash256 salt)
	{
	Span<byte> input = stackalloc byte[32 + 32];
	sourceAddress.Bytes.CopyTo(input.Slice(32 - Address.Size, Address.Size));
	salt.Bytes.CopyTo(input.Slice(32));
	return ValueKeccak.Compute(input);
	}
	// Mirrors how the RECENT_ROOT_ADDRESS predeploy derives source_id, so the read-path assertions above have
	// realistic inputs. Consensus-critical and spec-ambiguous: the 32-byte-padded address matches the only
	// existing implementation, while the spec text says 20 bytes.
	private static ValueHash256 SourceId(Address sourceAddress, in ValueHash256 salt)
	{
	Span<byte> input = stackalloc byte[Hash256.Size + Hash256.Size];
	sourceAddress.Bytes.CopyTo(input.Slice(Hash256.Size - Address.Size, Address.Size));
	salt.Bytes.CopyTo(input.Slice(Hash256.Size));
	return ValueKeccak.Compute(input);
	}

[claude]

Medium — the stated justification doesn't hold on this branch.

The PR description says the read path "is called by the reference-validity check on the execution path" and that "Block processing installs the predeploy code (PredeployInstaller)". Neither is true at this commit:

• IsReferenceValid has zero callers outside RecentRootStoreTests.
• There is no PredeployInstaller type anywhere in src/Nethermind.
• Nothing outside this file references Eip8272Constants.RecentRootAddress; Eip8272Constants.MaxRecentRootReferences has no references at all.

So the whole type is currently unreachable from production — this PR removes one dead method from a dead class, rather than the dead path from a live one. The removal itself is safe (I'm not arguing to keep Write), but the asymmetric treatment needs a truthful reason. Either:

1. state that the read path is kept as groundwork for the follow-up that wires frame-transaction validation (fine, and matches how Eip8272Constants reads — "Provisional: spec address is TBD"), or
2. if no follow-up is imminent, AGENTS.md ("When multiple solutions are viable, prefer … one that removes code"; "If a change makes existing code unused, remove it") points at dropping RecentRootStore + Eip8272Constants entirely.

Same applies to this <remarks>: "written by the RECENT_ROOT_ADDRESS predeploy bytecode during ordinary execution, not by the client" is accurate as spec description, but as written it implies Nethermind installs and executes that predeploy today, which it does not.

[github-actions]

EVM Opcode Benchmark Diff

Aggregated runs: base=3, pr=3
Noisy rerun opcodes: LOG0

No significant regressions or improvements detected.