Extra null check for V1 genesis

[ak88]

as per title

[claude]

Claude finished @ak88's task in 1m 48s —— View job

---

PR Review: Extra null check for V1 genesis

• Gathering context
• Reading changed files and rules
• Analyzing the change
• Posting review

---

Summary

This is a one-line defensive fix in QuorumCertificateManager.Initialize to prevent a NullReferenceException when the node starts up with its head at a V1 block (a block that has no ExtraConsensusData). The fix is correct and safe in the happy path.

Two issues are worth addressing before merge.

---

Medium: Missing regression test

The AGENTS.md guidelines require a regression test for every bug fix:

"When fixing a bug, always add a regression test"

No test was added covering Initialize called with a block where ExtraConsensusData is null and Number != spec.SwitchBlock. QuorumCertificateManagerTest.cs has good test infrastructure — a new test case there would slot in easily.

The test should verify that:

• _context.HighestQC is set to a genesis QC pointing to the V1 block
• _context.CurrentRound is set to 1
• No exception is thrown

Fix this →

---

Low: Silent handling of unexpected null — missing log

When ExtraConsensusData is null but Number > spec.SwitchBlock, the node silently resets to round 1. If this ever happens for a V2 block due to DB corruption or a serialization bug, the silent fallback makes the issue invisible.

A single _logger.Warn (or at minimum _logger.Info) on the null path would make this observable:

if (current.Number == spec.SwitchBlock || current.ExtraConsensusData is null)
{
    if (current.Number != spec.SwitchBlock)
        if (_logger.IsWarn) _logger.Warn($"Block #{current.Number} has no V2 consensus data, initializing from V1 genesis.");
    // ...
}

---

Code correctness — no issues

The logic is sound:

• ExtraConsensusData returns null for V1 blocks (no version byte match or no extra data) — the null check is the right guard.
• The genesis QC constructed uses current.Number and the gap relative to the V1 head, which is appropriate for bootstrapping from a mid-chain V1 block.
• The else branch (CommitCertificate(current.ExtraConsensusData.QuorumCert)) is only reached when ExtraConsensusData is non-null, so no NRE remains.

[claude]

Claude finished @ak88's task in 1m 14s —— View job

---

Review

• Gather context
• Read changed file
• Review code changes
• Post findings

---

Summary

The fix is correct: without the null check, Initialize would throw a NullReferenceException on any block where ExtraConsensusData is null and current.Number != spec.SwitchBlock (e.g. a V1 genesis block whose number doesn't match SwitchBlock). The one-line change is minimal and properly scoped.

One medium-severity issue:

Medium — Silent fallback when ExtraConsensusData is null on a non-genesis block

When the new || current.ExtraConsensusData is null arm fires for a block that is not the switch block, the node silently resets consensus to round 1 with a synthetic zero-round QC. CommitBlock (line 95) handles the same missing-data condition with a warning log ("Chain might be corrupt!"); Initialize should be consistent. Without a log here, chain corruption or a bad deserialization that leaves ExtraConsensusData null on a mid-chain block produces no observable signal.

See the inline comment for a suggested one-liner fix.

---

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

[claude]

Medium — Silent fallback without a warning log

When current.ExtraConsensusData is null on a block that is not the switch block, Initialize silently treats it as a genesis block: round is reset to 1 and a synthetic zero-round QC is constructed. This is the correct action to avoid an NPE (the original bug), but it is inconsistent with how the rest of the file handles the same condition — CommitBlock at line 95 logs a warning and returns an error for the same missing-data scenario ("Chain might be corrupt!").

Without a warning log here, chain corruption or a deserialization bug that leaves ExtraConsensusData null on a mid-chain block will produce no observable signal; the node will just silently restart consensus from round 1.

Suggested fix — add a warn log inside the branch when it isn't the switch block:

Suggested change

	if (current.Number == spec.SwitchBlock || current.ExtraConsensusData is null)
	if (current.Number == spec.SwitchBlock || current.ExtraConsensusData is null)
	{
	if (current.Number != spec.SwitchBlock && _logger.IsWarn)
	_logger.Warn($"Block {current.ToString(BlockHeader.Format.FullHashAndNumber)} has no consensus data; initializing consensus from genesis state.");

Fix this →

[claude]

The null guard is correct and prevents the NRE in the else branch. Two suggestions:

1. Regression test — please add a test in QuorumCertificateManagerTest covering Initialize with a V1 block (ExtraConsensusData is null, Number != spec.SwitchBlock). The existing test helpers make this straightforward.

2. Observability — a _logger.Warn on the null path (when Number != spec.SwitchBlock) would make it visible if this is ever hit unexpectedly for a V2 block due to serialization or DB issues, rather than silently resetting to round 1.

Suggested change

	if (current.Number == spec.SwitchBlock || current.ExtraConsensusData is null)
	if (current.Number == spec.SwitchBlock || current.ExtraConsensusData is null)
	{
	if (current.ExtraConsensusData is null && current.Number != spec.SwitchBlock)
	if (_logger.IsWarn) _logger.Warn($"Block #{current.Number} ({current.Hash}) has no V2 consensus data, initializing consensus from V1 genesis.");

[ak88]

@claude again

[claude]

Claude finished @ak88's task in 4m 18s —— View job

---

Implemented review suggestions

• Gather context
• Read QuorumCertificateManager.cs and find the Initialize method
• Add warning log for null ExtraConsensusData on non-switch-block genesis
• Add regression test for V1 genesis block initialization
• Commit and push

---

Two changes pushed to the branch:

1. Observability log in QuorumCertificateManager.Initialize

Added a Warn log when ExtraConsensusData is null but the block is not the switch block (i.e., initializing from a V1 genesis block). This matches the pattern in CommitBlock, which already logs a warning for the same missing-data condition.

2. Regression test Initialize_GenesisBlockWithNoConsensusData_SetsHighestQCAndRound

Added to QuorumCertificateManagerTest.cs. It constructs a genesis block (Number=0, no ExtraData, hence ExtraConsensusData is null) with a spec where SwitchBlock=900, calls Initialize, and asserts:

• No exception thrown
• HighestQC is set pointing to the genesis block at round 0
• CurrentRound is 1
  ·