Fix multiple headers not being referenced #7581

sstanculeanu · 2025-12-23T13:41:33Z

Reasoning behind the pull request

when some pending mini blocks from a different meta were added into the proposal, after selecting the outgoing transactions, the second meta header was not referenced on the proposal

Proposed changes

save more pending mini blocks and reference all new meta headers where needed

Testing procedure

with feat branch

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?

…ed after tx selection

…ersx/mx-chain-go into fix-multiple-headers-not-being-referenced # Conflicts: # process/block/gasConsumption.go # testscommon/gasComputationMock.go

codecov · 2025-12-23T14:24:43Z

Codecov Report

❌ Patch coverage is 83.07692% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.66%. Comparing base (49af3c3) to head (75782c2).
⚠️ Report is 11 commits behind head on feat/supernova-async-exec.

Files with missing lines	Patch %	Lines
process/block/gasConsumption.go	80.64%	4 Missing and 2 partials ⚠️
process/block/shardblockProposal.go	83.87%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@                    Coverage Diff                     @@
##           feat/supernova-async-exec    #7581   +/-   ##
==========================================================
  Coverage                      77.66%   77.66%           
==========================================================
  Files                            874      874           
  Lines                         120635   120689   +54     
==========================================================
+ Hits                           93694    93739   +45     
- Misses                         20768    20777    +9     
  Partials                        6173     6173

☔ 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.

…t-being-referenced

Copilot

Pull request overview

This PR fixes an issue where multiple meta headers were not being properly referenced in proposals when pending mini blocks from different meta blocks were added. The fix ensures that when outgoing transactions are selected and pending mini blocks are included, all corresponding meta headers are tracked and referenced.

Key Changes:

Added a new method CanAddPendingIncomingMiniBlocks() to the GasComputation interface to check if more pending mini blocks can fit in the remaining block space
Introduced pendingMiniBlocksAfterSelection struct to associate pending mini blocks with their originating meta headers
Enhanced the logic to reference all meta headers when pending mini blocks are added after transaction selection
Added comprehensive test coverage for the new gas computation check

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
process/interface.go	Added `CanAddPendingIncomingMiniBlocks()` method to GasComputation interface
testscommon/gasComputationMock.go	Mock implementation of the new interface method
process/coordinator/processProposal.go	Fixed transaction count calculation when mini blocks become pending; corrected typo in comment
process/coordinator/processProposal_test.go	Updated comment typo and adjusted expected transaction count in test
process/block/shardblockProposal.go	Core fix: tracks pending mini blocks with their headers and references all headers when pending blocks are added
process/block/shardblockProposal_test.go	Enhanced test to verify multiple meta headers are referenced correctly
process/block/gasConsumption.go	Implemented pending gas tracking and the new `CanAddPendingIncomingMiniBlocks()` method
process/block/gasConsumption_test.go	Added comprehensive tests for the new gas computation functionality
process/block/export_test.go	Exported internal types and methods for testing

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

Copilot · 2025-12-29T11:18:45Z

process/block/shardblockProposal.go

+		pendingMiniBlocks = append(pendingMiniBlocks, &pendingMiniBlocksAfterSelection{
+			headerHash:                 currentMetaBlockHash,
+			header:                     currentMetaBlock,
+			pendingMiniBlocksAndHashes: createIncomingMbsResult.PendingMiniBlocks,
+		})


The code unconditionally appends to pendingMiniBlocks even when createIncomingMbsResult.PendingMiniBlocks is empty. This could result in appending unnecessary empty entries to the slice. Consider only appending when there are actual pending mini blocks to track.

Suggested change

pendingMiniBlocks = append(pendingMiniBlocks, &pendingMiniBlocksAfterSelection{

headerHash: currentMetaBlockHash,

header: currentMetaBlock,

pendingMiniBlocksAndHashes: createIncomingMbsResult.PendingMiniBlocks,

})

if len(createIncomingMbsResult.PendingMiniBlocks) > 0 {

pendingMiniBlocks = append(pendingMiniBlocks, &pendingMiniBlocksAfterSelection{

headerHash: currentMetaBlockHash,

header: currentMetaBlock,

pendingMiniBlocksAndHashes: createIncomingMbsResult.PendingMiniBlocks,

})

}

should be ok

AdoAdoAdo · 2025-12-29T11:20:58Z

process/block/gasConsumption.go

 				hashStr := string(mb.GetHash())
 				gc.transactionsForPendingMiniBlocks[hashStr] = transactions[hashStr]
+
+				transactionsForMB, found := transactions[string(mb.GetHash())]


can you merge this line and L128?

both appear to be reading the same transactions

e.g do the gc.transactionsForPendingMiniBlocks[hashStr] = transactionsForMB after the !found check?
or do we need to set nil explicitly in the map?

updated, missed it

AdoAdoAdo · 2025-12-29T11:25:37Z

process/block/gasConsumption.go

+
+		gasConsumedByIncomingAfter := gc.totalGasConsumed[incoming]
+		gasConsumedByMiniBlock := gasConsumedByIncomingAfter - gasConsumedByIncomingBefore
+		if gasConsumedByMiniBlock < gc.totalGasConsumed[pending] {


for the last pending miniblock I think this condition would be equal.

this relates to the comment below

// should never be false

but for the last one, it will be false.

maybe the conditon should be <= gc.totalGasConsumed[pending]

AdoAdoAdo · 2025-12-29T11:37:56Z

process/block/gasConsumption.go

+				gasConsumedByPendingMb, errCheckPending := gc.checkGasConsumedByMiniBlock(mb, transactionsForMB)
+				if errCheckPending != nil {
+					log.Warn("failed to check gas consumed by pending mini block", "hash", mb.GetHash(), "error", errCheckPending)
+					continue


I think it should break instead of continuing, as that means that it can create miniblock gaps when there's a problem, which would not be ok.

Same with the above continue, I think it should break there as well, as we don't want to create gaps.

Or even better returning error instead of breaking, as we rely now on the computed pending miniblocks consumed gas in the next steps.

right, updated

AdoAdoAdo · 2025-12-29T12:24:00Z

process/block/shardblockProposal.go

+	}
+
+	for referencedHash, referencedHeader := range extraHeadersReferenced {
+		sp.miniBlocksSelectionSession.AddReferencedHeader(referencedHeader, []byte(referencedHash))


the miniblockSelectionSession assumes the referenced headers are added in order, and here you are iterating over a map which does not ensure a specific iteration order.

Can you ensure that the AddReferencedHeader is called in the order of referencing the metablocks? maybe adding the reference into the above loop?

updated as suggested

AdoAdoAdo · 2025-12-29T12:36:29Z

process/block/gasConsumption.go

+
+	gasLeft := maxGasLimitPerBlock - totalGasConsumed
+	maxGasLimitPerMiniBlock := gc.maxGasLimitPerMiniBlock(gc.shardCoordinator.SelfId())
+	return totalGasToBeConsumedByPending+maxGasLimitPerMiniBlock < gasLeft


why do we add the maxGasLimitPerMiniBlock to the total gas to be consumed by pending?

this method checks if there is space for one more mini block left in the block.. checking against the max gas limit, as it does not receive a specific mini block to know how much it would consume

but the miniblock does not necessarily need to be of max size, it can be of any size below the maximum size.

AdoAdoAdo · 2025-12-29T12:48:39Z

process/block/shardblockProposal.go

+	return nil
+}
+
+func getIndexOfPendingMiniBlock(


this is called getIndexOfPending.... but it is not getting any index right?

…t-being-referenced

fix: missing referenced meta header when pending mini blocks were add…

1d3b079

…ed after tx selection

sstanculeanu self-assigned this Dec 23, 2025

sstanculeanu added 2 commits December 23, 2025 15:50

Merge branch 'feat/supernova-async-exec' of https://github.com/multiv…

2fe6050

…ersx/mx-chain-go into fix-multiple-headers-not-being-referenced # Conflicts: # process/block/gasConsumption.go # testscommon/gasComputationMock.go

fix failing test

d0b998f

Merge branch 'feat/supernova-async-exec' into fix-multiple-headers-no…

4679907

…t-being-referenced

AdoAdoAdo requested review from AdoAdoAdo and Copilot December 29, 2025 11:13

Copilot started reviewing on behalf of AdoAdoAdo December 29, 2025 11:13 View session

Copilot AI reviewed Dec 29, 2025

View reviewed changes

AdoAdoAdo reviewed Dec 29, 2025

View reviewed changes

sstanculeanu and others added 3 commits December 29, 2025 15:29

fixes after review

ed2a868

Merge branch 'feat/supernova-async-exec' into fix-multiple-headers-no…

ade3ae0

…t-being-referenced

fixes after review

6b421eb

AdoAdoAdo approved these changes Dec 29, 2025

View reviewed changes

sstanculeanu added 2 commits December 29, 2025 18:14

Merge branch 'feat/supernova-async-exec' into fix-multiple-headers-no…

37e76be

…t-being-referenced

Merge branch 'feat/supernova-async-exec' into fix-multiple-headers-no…

75782c2

…t-being-referenced

ssd04 approved these changes Dec 30, 2025

View reviewed changes

sstanculeanu merged commit 80647ed into feat/supernova-async-exec Dec 30, 2025
13 checks passed

sstanculeanu deleted the fix-multiple-headers-not-being-referenced branch December 30, 2025 08:28

Fix multiple headers not being referenced #7581

Fix multiple headers not being referenced #7581

Uh oh!

Conversation

sstanculeanu commented Dec 23, 2025

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Uh oh!

codecov bot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 23, 2025 •

edited

Loading