fix(consensus): PrevHash validation silently skipped on empty input

[arepala-uml]

1. Made changes to PrevHeaderHash for all non-genesis blocks by returning an explicit error when it is empty, preventing silent prev-hash validation skipping.
2. Added tests for empty non-genesis (fail), valid non-genesis (pass), and genesis without PrevHeaderHash (pass).

Closes #1576

---

Summary by cubic

Require PrevHeaderHash for non-genesis blocks in consensus header validation so empty input no longer skips prev-hash checks. Closes #1576.

• Bug Fixes

  • Error when BlockNumber > 0 and PrevHeaderHash is missing; allow genesis without it.
  • Tests cover missing/matching non-genesis, genesis without prev hash, and mismatched hashes.

• Migration

  • Ensure callers set PrevHeaderHash for all non-genesis headers.

^{Written for commit 67e703e. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Strengthened block validation to require previous header hash for all non-genesis blocks, preventing incomplete or malformed block submissions and ensuring data integrity across the system.

• Tests

  • Updated test cases to enforce that non-genesis blocks properly require previous header hash, including error handling for missing references and validation of hash-matching scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request adds validation to the validatePrevHash function in consensus/validate.go to explicitly require the PrevHeaderHash field for all non-genesis blocks. When a non-genesis block (BlockNumber > 0) has an empty PrevHeaderHash, the validation now returns an error. The existing validation logic that compares PrevHash to PrevHeaderHash is retained when PrevHeaderHash is provided. Corresponding test cases in consensus/validate_test.go are updated to verify this behavior: a non-genesis block with empty PrevHeaderHash fails validation, a non-genesis block with matching PrevHash and PrevHeaderHash passes validation, and a genesis block without PrevHeaderHash passes validation.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All requirements from issue #1576 are met: PrevHeaderHash validation enforced for non-genesis blocks [#1576], explicit error returned [#1576], and all three test cases added [#1576].
Out of Scope Changes check	✅ Passed	All changes directly address issue #1576 requirements; only consensus/validate.go and its test file were modified with no extraneous alterations.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding validation to prevent PrevHash validation from being silently skipped when input is empty for non-genesis blocks.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch prehash_validation

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 2 files

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

consensus/validate_test.go (1)

152-197: ⚠️ Potential issue | 🟡 Minor

Use require assertions and validate the exact error message.

This test should use the testify/require assertions already imported in the file. Lines 159, 170, and 181 currently use t.Error and t.Errorf instead.

For the non-genesis missing hash case (line 159), use require.EqualError to validate both that an error occurred and that it matches the expected message. For the valid cases (lines 170, 181), use require.NoError:

Suggested update

 	err := validator.validatePrevHash(input)
-	if err == nil {
-		t.Error("expected error for missing previous header hash on non-genesis block")
-	}
+	require.EqualError(
+		t,
+		err,
+		"previous header hash is required for non-genesis blocks",
+	)

 	// Valid: non-genesis block with matching previous header hash
 	input = &ValidateHeaderInput{
 		PrevHash:       hash,
 		PrevHeaderHash: hash,
 		BlockNumber:    1,
 	}
 	err = validator.validatePrevHash(input)
-	if err != nil {
-		t.Errorf("expected no error for matching hashes, got %v", err)
-	}
+	require.NoError(t, err)

 	// Valid: genesis block without previous header hash
 	input = &ValidateHeaderInput{
 		PrevHash:       hash,
 		PrevHeaderHash: nil,
 		BlockNumber:    0,
 	}
 	err = validator.validatePrevHash(input)
-	if err != nil {
-		t.Errorf("expected no error for genesis block without prev hash, got %v", err)
-	}
+	require.NoError(t, err)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@consensus/validate_test.go` around lines 152 - 197, Replace the
t.Error/t.Errorf assertions in the validatePrevHash tests with testify/require
assertions: for the non-genesis missing previous header hash case (the
ValidateHeaderInput with BlockNumber 1 and PrevHeaderHash nil) use
require.EqualError(t, err, "<expected error message>") to assert an error and
its exact message; for the two valid cases (BlockNumber 1 with matching
PrevHeaderHash and BlockNumber 0 genesis case with PrevHeaderHash nil) use
require.NoError(t, err) to assert success; keep the same inputs (PrevHash,
PrevHeaderHash, BlockNumber) and the call to validator.validatePrevHash so only
the assertions change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@consensus/validate_test.go`:
- Around line 152-197: Replace the t.Error/t.Errorf assertions in the
validatePrevHash tests with testify/require assertions: for the non-genesis
missing previous header hash case (the ValidateHeaderInput with BlockNumber 1
and PrevHeaderHash nil) use require.EqualError(t, err, "<expected error
message>") to assert an error and its exact message; for the two valid cases
(BlockNumber 1 with matching PrevHeaderHash and BlockNumber 0 genesis case with
PrevHeaderHash nil) use require.NoError(t, err) to assert success; keep the same
inputs (PrevHash, PrevHeaderHash, BlockNumber) and the call to
validator.validatePrevHash so only the assertions change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d4af235c-73f5-40f6-bb81-377df77a7bf5

📥 Commits

Reviewing files that changed from the base of the PR and between 7e7cbce and 3fbd55f.

📒 Files selected for processing (2)

• consensus/validate.go
• consensus/validate_test.go