XDC Blockhash Store

[batrr]

Changes

• Add XdcBlockhashStore (IBlockhashStore for XDC) that deploys the EIP-2935 history contract on demand when it's missing, then records the parent block hash.
• Expose the history-contract bytecode as Eip2935Constants.Code.
• Make BlockhashStore.ApplyBlockhashStateChanges virtual so XDC can override it.
• Register XdcBlockhashStore in XdcModule; add tests.

Why

On Ethereum the EIP-2935 contract is deployed via a normal tx just before the fork, so the stock store assumes it already exists. XDC skips that step, so XdcBlockhashStore deploys it lazily (nonce=1) when the account has no code — reusing existing code if present, throwing on a code mismatch.

Types of changes

• New feature

Testing

• Yes / wrote tests — deploy, no-redeploy, code mismatch, genesis, EIP disabled, missing parent.

[claude]

Claude finished @batrr's task in 3m 19s —— View job

---

Review: XDC Blockhash Store

Reviewed correctness, edge cases, DI wiring, and hot-path performance. The lazy-deploy approach for the EIP-2935 history contract is sound and the test coverage of the store logic is good. However, there is one Critical DI wiring bug and a Medium hot-path concern. Inline comments posted on the relevant lines.

🔴 Critical

1. IBlockhashStore registered as Singleton but depends on scoped IWorldState — XdcModule.cs:71

.AddSingleton<IBlockhashStore, XdcBlockhashStore>() is a captive-dependency bug. IWorldState is AddScoped and the base wires the store as AddScoped<IBlockhashStore, BlockhashStore>() (BlockProcessingModule.cs:67). As a singleton, XdcBlockhashStore captures a single IWorldState for the node's lifetime, so ApplyBlockhashStateChanges (deploy + parent-hash write) runs against the wrong/leaked world state instead of the per-block scope that gets committed — an incorrect state root / invalid block once EIP-2935 activates on XDC, plus cross-block state leakage. Fix: use AddScoped. The unit tests construct the store directly, so they don't exercise the DI lifetime. Fix this →

🟠 Medium

2. LINQ + redundant full-bytecode compare every block — XdcBlockhashStore.cs:25

code.SequenceEqual(Eip2935Constants.Code) resolves to Enumerable.SequenceEqual (LINQ, disallowed by repo style, allocates enumerators). Once deployed, this override calls GetCode and re-scans the whole bytecode on every block. Compare code hashes instead (GetCodeHash vs a precomputed static readonly ValueKeccak.Compute(Eip2935Constants.Code)), or at minimum code.AsSpan().SequenceEqual(...). See inline suggestion.

🟡 Low

• SetNonce(account, 1) can clobber a pre-existing nonce — XdcBlockhashStore.cs:30-31: if the target address already exists as a non-code account with a nonce (e.g. it received funds / sent txs), the code.Length == 0 branch overwrites its nonce to 1. Extremely unlikely at this fixed address, but worth a comment noting the assumption.
• Doc-comment typo — Eip2935Constants.cs:27: stray /// (/// /// </summary>).
• Test style — XdcBlockhashStoreTests.cs lines 31, 50, 71, 95 have a double semicolon (;;); the per-test world-state/spec/header setup is also duplicated across all six tests and could be factored into a helper per the repo's test-DRY guidance.
• Redundant field — XdcBlockhashStore.cs:16: _worldState duplicates the primary-constructor parameter worldState, which is already captured; the field can be dropped and worldState used directly.

✅ Looks good

• Eip2935Constants.Code matches the canonical EIP-2935 runtime bytecode; deploy uses nonce=1 per EIP-161.
• Address resolution (spec.Eip2935ContractAddress ?? …BlockHashHistoryAddress) is consistent with the base.
• Guard duplication in the override is necessary (deploy precedes the base call); reuse-if-present / throw-on-mismatch semantics are correct.
• Good edge-case test coverage: deploy, no-redeploy, mismatch, genesis, EIP disabled, missing parent.

---

Verdict: Not mergeable as-is — the Critical DI registration must be fixed (and ideally the Medium hot-path compare) before merge.

• Read changed files
• Analyze correctness & edge cases
• Check performance (hot path)
• Verify DI registration
• Post review

[batrr]

@claude re review

[claude]

Claude finished @batrr's task in 1m 11s —— View job

---

Re-review: XDC Blockhash Store

I re-checked the four files against my prior review. All previously flagged issues have been resolved.

Previously-flagged issues — status

#	Severity	Issue	Status
1	🔴 Critical	IBlockhashStore registered as Singleton (captive IWorldState)	✅ Fixed — XdcModule.cs:71 now .AddScoped<IBlockhashStore, XdcBlockhashStore>(), mirroring the base wiring
2	🟠 Medium	LINQ + full-bytecode compare every block	✅ Fixed — now compares code hashes via GetCodeHash against a precomputed static readonly ValueHash256 CodeHash (XdcBlockhashStore.cs:15,25-28); no per-block byte scan, no LINQ
3	🟡 Low	Doc-comment typo (/// ///)	✅ Fixed — Eip2935Constants.cs:25-27 clean
4	🟡 Low	Test double-semicolons (;;)	✅ Fixed — none remain
5	🟡 Low	SetNonce(account, 1) can clobber a pre-existing nonce	🟡 Still present (XdcBlockhashStore.cs:33), but only in the !hasCode branch, at a fixed deterministic address — acceptable. Optionally worth a one-line comment noting the assumption.
6	🟡 Low	Redundant _worldState field	ℹ️ Kept intentionally — the base's worldState primary-ctor parameter is private, so the derived override cannot reach it; a local capture is required either way. No action needed.

Verification notes

• DI lifetime: XdcBlockhashStore → IWorldState (scoped) is now consistent — the store resolves per block-processing scope, so deploy + parent-hash writes land on the state root that gets committed. ✅
• Correctness: hash-based guard preserves reuse-if-present / throw-on-mismatch semantics; empty-code-hash (Keccak.OfAnEmptyString) correctly treated as "no code → deploy". Deploy uses nonce=1 per EIP-161. ✅
• Tests: six edge cases retained (deploy, store-parent-hash, no-redeploy, mismatch, genesis, EIP-disabled, missing-parent); Does_not_redeploy_... asserts the existing nonce is preserved. ✅ (A test for the account-exists-with-nonce-but-no-code clobber path from Patricia alt 2 #5 is still absent, but that path is not reachable in practice.)

Verdict

Mergeable. The Critical DI bug and the Medium hot-path concern are both resolved; only optional cosmetic nits (#5 comment) remain. Already has a maintainer approval.
· feat/xdc-blockhash

[ak88]

you can also pass nonce in this call

[asdacap]

Should this be in chainspec?