Skip to content

Merge rc into feat/supernova async exec 2026.04.02#7818

Merged
AdoAdoAdo merged 8 commits intofeat/supernova-async-execfrom
merge-rc-into-feat/supernova-async-exec-2026.04.02
Apr 2, 2026
Merged

Merge rc into feat/supernova async exec 2026.04.02#7818
AdoAdoAdo merged 8 commits intofeat/supernova-async-execfrom
merge-rc-into-feat/supernova-async-exec-2026.04.02

Conversation

@sstanculeanu
Copy link
Copy Markdown
Collaborator

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

AdoAdoAdo and others added 6 commits April 1, 2026 20:34
add extra validation
…to rc/supernova

# Conflicts:
#	process/block/baseProcess.go
#	process/errors.go
…va-2026.04.02

Merge master into rc/supernova 2026.04.02
 into feat/supernova-async-exec

# Conflicts:
#	process/block/baseProcess.go
#	process/block/interceptedBlocks/interceptedMetaBlockHeader.go
@sstanculeanu sstanculeanu self-assigned this Apr 2, 2026
@sstanculeanu sstanculeanu added the ignore-for-release-notes Do not include item in release notes label Apr 2, 2026
AdoAdoAdo
AdoAdoAdo previously approved these changes Apr 2, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds validation for duplicated mini-block (and shard-data) hashes to prevent blocks/headers containing repeated hashes where uniqueness is expected, and introduces a dedicated error for this condition.

Changes:

  • Added process.ErrDuplicatedHashInBlock error.
  • Added duplicate-hash detection in intercepted meta-header integrity checks and in header/body correlation checks.
  • Added/updated unit tests for duplicated hashes across shard processors and intercepted meta headers.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
process/errors.go Introduces ErrDuplicatedHashInBlock used by new validation paths.
process/block/baseProcess.go Adds duplicate-hash detection to header/body correlation logic.
process/block/shardblock_test.go Adds tests asserting correlation fails on duplicated hashes.
process/block/interceptedBlocks/common.go Adds shared helper + enforces uniqueness for shard-info and miniblock header hashes.
process/block/interceptedBlocks/common_test.go Adds unit tests for duplicate-hash checks in intercepted-block utilities.
process/block/interceptedBlocks/interceptedMetaBlockHeader.go Minor formatting change in integrity flow (no logic change).
process/block/interceptedBlocks/interceptedMetaBlockHeader_test.go Adds tests ensuring duplicated miniblock header hashes are rejected.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// check if header has the same mini blocks as presented in body
func (bp *baseProcessor) checkHeaderBodyCorrelationProposal(miniBlockHeaders []data.MiniBlockHeaderHandler, body *block.Body) error {
mbHashesFromHdr := make(map[string]data.MiniBlockHeaderHandler, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkHeaderBodyCorrelationProposal builds mbHashesFromHdr by calling miniBlockHeaders[i].GetHash() before validating that the header handler is non-nil. If miniBlockHeaders contains a nil entry (there is already a unit test for this case), this will panic with a nil pointer dereference instead of returning ErrNilMiniBlockHeader. Consider validating miniBlockHeaders entries before building the map, or building the map inside the main loop after the nil check.

Suggested change
for i := 0; i < len(miniBlockHeaders); i++ {
for i := 0; i < len(miniBlockHeaders); i++ {
if miniBlockHeaders[i] == nil {
return process.ErrNilMiniBlockHeader
}

Copilot uses AI. Check for mistakes.
Comment on lines +1152 to +1154
mbHashesFromHdr := make(map[string]data.MiniBlockHeaderHandler, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
mbHashesFromHdr[string(miniBlockHeaders[i].GetHash())] = miniBlockHeaders[i]
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mbHashesFromHdr is only used for presence checks and deletions; the stored MiniBlockHeaderHandler value is never read. Using map[string]struct{} (or a boolean) would clarify intent and avoid keeping unused pointers in the map.

Suggested change
mbHashesFromHdr := make(map[string]data.MiniBlockHeaderHandler, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
mbHashesFromHdr[string(miniBlockHeaders[i].GetHash())] = miniBlockHeaders[i]
mbHashesFromHdr := make(map[string]struct{}, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
mbHashesFromHdr[string(miniBlockHeaders[i].GetHash())] = struct{}{}

Copilot uses AI. Check for mistakes.
Comment on lines +1233 to +1235
mbHashesFromHdr := make(map[string]data.MiniBlockHeaderHandler, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
mbHashesFromHdr[string(miniBlockHeaders[i].GetHash())] = miniBlockHeaders[i]
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mbHashesFromHdr is only used as a set (membership + delete); the map value is never read. Consider changing this to map[string]struct{} to make the code’s intent clearer and reduce memory overhead.

Suggested change
mbHashesFromHdr := make(map[string]data.MiniBlockHeaderHandler, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
mbHashesFromHdr[string(miniBlockHeaders[i].GetHash())] = miniBlockHeaders[i]
mbHashesFromHdr := make(map[string]struct{}, len(miniBlockHeaders))
for i := 0; i < len(miniBlockHeaders); i++ {
mbHashesFromHdr[string(miniBlockHeaders[i].GetHash())] = struct{}{}

Copilot uses AI. Check for mistakes.
raduchis
raduchis previously approved these changes Apr 2, 2026
@sstanculeanu sstanculeanu dismissed stale reviews from raduchis and AdoAdoAdo via e6d3488 April 2, 2026 10:36
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 91.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.58%. Comparing base (3f8d81a) to head (2dbcd45).
⚠️ Report is 14 commits behind head on feat/supernova-async-exec.

Files with missing lines Patch % Lines
process/block/baseProcess.go 83.33% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           feat/supernova-async-exec    #7818   +/-   ##
==========================================================
  Coverage                      77.58%   77.58%           
==========================================================
  Files                            882      882           
  Lines                         123941   123987   +46     
==========================================================
+ Hits                           96155    96193   +38     
- Misses                         21416    21420    +4     
- Partials                        6370     6374    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AdoAdoAdo AdoAdoAdo merged commit fe99fe7 into feat/supernova-async-exec Apr 2, 2026
10 checks passed
@AdoAdoAdo AdoAdoAdo deleted the merge-rc-into-feat/supernova-async-exec-2026.04.02 branch April 2, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore-for-release-notes Do not include item in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants