Enable nullables in 6 base projects

[flcl42]

Changes

• Enabled nullable reference types for the first bottom-up foundation batch: Crypto, Db, Network.Stats, Serialization.Json, Serialization.Rlp, and Specs.
• Propagated the sharper contracts through dependent runtime, protocol, benchmark, and test code that consumes those projects.
• Tightened RLP null handling for mandatory header fields, hash arrays, nullable block bodies, and delegate-based array decoding of value-type elements.
• Added regression coverage for delegate-based RLP array decoding so empty-list elements no longer become value-type defaults.

Visual summary

Green nodes show the new nullable-aware state. Red nodes show replaced/deleted assumptions. Blue nodes show contracts that became stricter non-null.

flowchart LR
    classDef old fill:#fee2e2,stroke:#dc2626,color:#111827
    classDef added fill:#dcfce7,stroke:#16a34a,color:#052e16

    CryptoOld["Nethermind.Crypto<br/>Nullable: annotations"] --> CryptoNew["Nethermind.Crypto<br/>Nullable: enable"]
    DbOld["Nethermind.Db<br/>Nullable: annotations"] --> DbNew["Nethermind.Db<br/>Nullable: enable"]
    StatsOld["Nethermind.Network.Stats<br/>Nullable: disabled"] --> StatsNew["Nethermind.Network.Stats<br/>Nullable: enable"]
    JsonOld["Nethermind.Serialization.Json<br/>Nullable: disabled"] --> JsonNew["Nethermind.Serialization.Json<br/>Nullable: enable"]
    RlpOld["Nethermind.Serialization.Rlp<br/>Nullable: annotations"] --> RlpNew["Nethermind.Serialization.Rlp<br/>Nullable: enable"]
    SpecsOld["Nethermind.Specs<br/>Nullable: annotations"] --> SpecsNew["Nethermind.Specs<br/>Nullable: enable"]

    class CryptoOld,DbOld,StatsOld,JsonOld,RlpOld,SpecsOld old
    class CryptoNew,DbNew,StatsNew,JsonNew,RlpNew,SpecsNew added

Loading

flowchart TD
    classDef added fill:#dcfce7,stroke:#16a34a,color:#052e16
    classDef removed fill:#fee2e2,stroke:#dc2626,color:#111827
    classDef strict fill:#dbeafe,stroke:#2563eb,color:#111827

    RlpOld["RLP old assumptions<br/>generic Decode/Encode and array helpers treated null/default loosely"] --> RlpNew["RLP nullable-aware contracts<br/>MaybeNull Decode, T? Encode/GetLength, nullable arrays explicit"]
    RlpOld2["Deleted: empty-list element could become default(T)<br/>in delegate DecodeArray and DecodeArrayPoolList"] --> RlpNew2["Added: null/default guard<br/>ThrowNullArrayElement for malformed non-null arrays"]
    RlpOld3["Deleted: allowNulls flag inside DecodeArray"] --> RlpNew3["Added: DecodeArray for non-null class arrays<br/>and DecodeNullableArray for nullable class arrays"]
    RlpStrict["Added non-null scalar decode helpers<br/>DecodeKeccakNonNull, DecodeZeroPrefixKeccakNonNull,<br/>DecodeAddressNonNull, DecodeBloomNonNull"]

    JsonOld["JSON old return shape<br/>Deserialize returned object / T"] --> JsonNew["JSON nullable return shape<br/>Deserialize returns object? / T?"]
    JsonOld2["Hash256Converter wrote only Hash256"] --> JsonNew2["Hash256Converter handles Hash256?<br/>and writes JSON null"]
    JsonOld3["ByteArrayArrayConverter handled byte[][]"] --> JsonNew3["ByteArrayArrayConverter handles byte[]?[]?<br/>outer and inner nulls are explicit"]

    DbOld["DB old shape<br/>column keys unconstrained; GetAll/Values carried nullable values"] --> DbNew["DB stricter shape<br/>column keys are notnull; GetAll/Values enumerate non-null persisted values"]
    PruneOld["Full pruning old out parameter<br/>out IPruningContext"] --> PruneNew["Full pruning Try pattern<br/>NotNullWhen(true) out IPruningContext?"]

    CryptoOld["ECIES old decrypt tuple<br/>byte[] PlainText even when Success=false"] --> CryptoNew["ECIES decrypt tuple<br/>byte[]? PlainText tied to Success"]
    StatsOld["Network.Stats old details<br/>P2P/Eth/Les details assumed present"] --> StatsNew["Network.Stats optional details<br/>P2PNodeDetails?, EthNodeDetails?, LesNodeDetails?"]
    SpecsOld["Chain spec old optional JSON fields<br/>modeled as non-null references"] --> SpecsNew["Chain spec nullable fields<br/>optional addresses, hashes, genesis, allocations, names are marked ?"]

    class RlpNew,RlpNew2,RlpNew3,RlpStrict,JsonNew,JsonNew2,JsonNew3,PruneNew,CryptoNew,StatsNew,SpecsNew added
    class RlpOld,RlpOld2,RlpOld3,JsonOld,JsonOld2,JsonOld3,DbOld,PruneOld,CryptoOld,StatsOld,SpecsOld removed
    class DbNew strict

Loading

flowchart LR
    classDef upgraded fill:#dcfce7,stroke:#16a34a,color:#052e16
    classDef consumer fill:#fef9c3,stroke:#ca8a04,color:#111827

    Crypto["Crypto"] --> Discovery["Network.Discovery consumers"]
    Crypto --> Hive["Hive/runtime consumers"]
    Db["Db"] --> DbRocks["Db.Rocks"]
    Db --> TxPool["TxPool BlobTxStorage"]
    Db --> PruningTests["pruning and DB tests"]
    Stats["Network.Stats"] --> DiscoveryAdapters["discv4/discv5 adapters"]
    Json["Serialization.Json"] --> JsonRpc["JsonRpc tests and serializers"]
    Json --> MergeTests["Merge.Plugin tests"]
    Rlp["Serialization.Rlp"] --> Network["Network eth/snap message serializers"]
    Rlp --> Era["Era1 reader"]
    Rlp --> Optimism["Optimism decoders"]
    Rlp --> Xdc["XDC header decoders"]
    Specs["Specs"] --> OptimismSpecs["Optimism/Taiko/Shutter chain-spec consumers"]

    class Crypto,Db,Stats,Json,Rlp,Specs upgraded
    class Discovery,Hive,DbRocks,TxPool,PruningTests,DiscoveryAdapters,JsonRpc,MergeTests,Network,Era,Optimism,Xdc,OptimismSpecs consumer

Loading

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: Nullable reference type annotations and contract tightening

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Notes on testing

• Main Nethermind solution Release build passed with warnings as errors.
• Repository Code Lint-shaped analyzer build passed.
• Benchmark solution Release build passed with warnings as errors.
• Focused RLP decoder regression fixture passed.
• Ethereum test-suite solution build is blocked in this checkout by missing external fixture JSON files under src/tests; no files under src/tests were changed.

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

Remarks

This is the first smaller nullable batch. Later nullable batches should continue upward through dependent runtime projects.

[claude]

Claude finished @flcl42's task in 8m 16s —— View job

---

Review: Enable foundation nullables (#12155)

• Gather context & diff
• Review RLP serialization null-handling changes
• Review Crypto changes (IES/ECDSA/KDF)
• Review Db changes (MemDb, SnapshotableMemDb, FullPruningDb, ReadOnly)
• Review Specs & remaining contract changes
• Post review summary

---

Overall this is a careful, well-scoped nullability batch. The core fix — splitting DecodeArray<T> into a strict class-constrained variant (DecodeGuardNotNull) and a DecodeNullableArray, so value-type array elements can no longer silently become default(T) for an empty-list (0xC0) element — is correct, consensus-relevant, and backed by new regression tests in RlpDecoderTests. Header/XDC-header mandatory fields now use DecodeKeccakNonNull/DecodeAddressNonNull/DecodeBloomNonNull, which is correct (those fields are always present, even in genesis). The EthereumIesEngine restructuring preserves the crypto logic and only adds init guards. DB nullability changes are consistent (MemDb.Set(null) deletes, so the byte[]-valued dictionary never holds null).

Findings

Medium

• Nethermind.Era1/EraReader.cs:195 — DecodeReceipts dropped the explicit allowNulls: true (and its comment), switching ERA1 receipt decoding from tolerant (null entry for an empty-list 0xC0 element) to throwing RlpException. ReceiptMessageDecoder really does return null for 0xC0 items, so this is a genuine semantic narrowing with no test on the ERA path. Almost certainly intentional and arguably better, but please confirm and add a short note (or use DecodeNullableArray if null-tolerance is actually required). inline

Low

• Discv5/Kademlia/KademliaAdapter.cs:694 — the new empty-enr early return runs before TryGetKnownRecord, so a node with a cached record but empty node.Enr now returns null (excluded from FINDNODE) instead of the cached record. Consider keeping the known-record lookup first. inline
• Nethermind.Db/MemDb.cs:87-88 — GetAll/OrderedDb now allocate a new KeyValuePair per entry (previously returned _db directly) purely to satisfy the non-nullable return type. Minor allocation cost on enumeration paths. inline
• Nethermind.Db/FullPruning/FullPruningDb.cs:FinishPruning — now early-returns when _pruningContext is null instead of Interlocked.Exchange(ref _currentDb, null) + ClearOldDb. This is safer (avoids nulling the live DB), but it is a behavior change; worth a one-line confirmation it is intended.
• Nethermind.TxPool/BlobTxStorage.cs:131 — return blockBlobTransactions is not null is effectively always true since RlpDecoder<T>.DecodeArray returns a non-null array; the [NotNullWhen(true)] guard is fine but the null check is dead. Harmless.

Positive notes

• RlpDecoder<T>.Encode(T?)/EncodeToCappedArray/GetNullableLength are internally consistent: null → 1-byte 0xC0 via EncodeNullObject, matching GetNullableLength.
• Discv4 KademliaAdapter now drops messages with null FarPublicKey/FarAddress instead of NRE'ing the Node constructor — good defensive improvement.
• Rlp static ctor typeof(Rlp).Assembly replacing Assembly.GetAssembly(typeof(Rlp)) is equivalent and cleaner.

Verdict: Not blocking on bugs, but the Medium ERA1 behavior change should be explicitly acknowledged (or covered/commented) before merge. I couldn't independently run the build/tests in this review environment; the PR reports Release builds passing with warnings-as-errors.

[claude]

[Medium] Behavior change: ERA1 receipt decoding switched from tolerant to throwing.

On master this was:

// ReceiptMessageDecoder returns null for empty-list (0xC0) items
return ctx.DecodeArray<TxReceipt>(_receiptDecoder, allowNulls: true);

The class-constrained DecodeArray<T> now uses DecodeGuardNotNull, and ReceiptMessageDecoder.DecodeInternal genuinely returns null for an empty-list (0xC0) element. So an ERA1 archive containing such an element now throws RlpException during import instead of producing a null array entry.

For well-formed canonical archives this is fine (receipts are never 0xC0), and arguably stricter-is-better. But the deliberate allowNulls: true plus its explanatory comment were load-bearing-looking, and there is no test asserting the new strict behavior on the ERA path. Please confirm this narrowing is intentional; if so, consider a one-line comment noting that empty-list receipt elements are now rejected. If null-tolerance is actually required here, use DecodeNullableArray instead.

[claude]

[Low] Reordering changes lookup behavior. The new early-return on string.IsNullOrEmpty(enr) runs before TryGetKnownRecord(node.Id.Hash, ...). Previously the cached-record lookup ran first, so a node that has a known/cached NodeRecord but an empty node.Enr string would have returned the cached record; now it returns null and is excluded from FINDNODE responses.

If a cached record can ever exist independently of node.Enr, this silently drops valid relays. Safer to keep the TryGetKnownRecord check first and only guard the empty-enr case around the FromEnrString call:

if (TryGetKnownRecord(node.Id.Hash, out NodeRecord? knownRecord))
    return IsAcceptableNodeRecord(...) ? knownRecord : null;

if (string.IsNullOrEmpty(node.Enr)) return null;

[claude]

[Low] Extra per-element allocation on enumeration. GetAll/OrderedDb previously returned _db (or its ordered view) directly; they now project each entry into a freshly-allocated KeyValuePair<byte[], byte[]> via Select. Since MemDb.Set(key, null) deletes rather than storing null, the values are guaranteed non-null, so the projection is purely to satisfy the non-nullable return type. Functionally correct, but it adds an allocation/copy per entry on every GetAll enumeration (used by pruning/copy-tree/visitor paths). Minor; acceptable for the nullability cleanup, just flagging the cost.