Reject proofs without header by sstanculeanu · Pull Request #7805 · multiversx/mx-chain-go

sstanculeanu · 2026-03-27T12:18:48Z

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?

… delayed

ssd04 · 2026-03-27T14:49:52Z

 	epoch := rrh.getEpoch()
-	rrh.RequestEquivalentProofByNonceForEpoch(headerShard, headerNonce, epoch)
+	go func(requestEpoch uint32) {
+		time.Sleep(rrh.requestProofByNonceDelay)


i'm thinking if it might affect requestHeadersIfSyncIsStuck - currently it requests by nonce up to 20 proofs each time; 100ms sleep per 20 requests should not be a delay issue; but maybe number of goroutines? at least to not forget the connection with MaxHeadersToRequestInAdvance const which may affect if we increase it

indeed this may grow

Copilot

Pull request overview

This PR tightens equivalent proof validation by rejecting proofs whose referenced header is not available locally, and adds an optional delay for proof-by-nonce requests to reduce the chance of requesting/receiving proofs before headers.

Changes:

Inject HeadersPool into the equivalent proofs interceptor path and validate header presence + round match during CheckValidity().
Add RequestProofByNonceDelayMs config and implement delayed RequestEquivalentProofByNonce scheduling.
Update chain simulator, integration wiring, and tests to account for the new dependency and delayed behavior.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
process/interceptors/factory/interceptedEquivalentProofsFactory_test.go	Updates factory test args to include `HeadersPool`.
process/interceptors/factory/interceptedEquivalentProofsFactory.go	Extends factory args/state to pass `HeadersPool` into intercepted equivalent proofs.
process/factory/interceptorscontainer/baseInterceptorsContainerFactory.go	Wires `HeadersPool` from datapool into equivalent proofs interceptor factory.
process/block/interceptedBlocks/interceptedEquivalentProof_test.go	Adds headers pool nil-check test and “missing header” validity test; updates mocks to include `HeadersPool`.
process/block/interceptedBlocks/interceptedEquivalentProof.go	Enforces “header must exist” and round match before accepting/storing proofs.
node/chainSimulator/configs/configs.go	Sets `RequestProofByNonceDelayMs` for simulator configs.
node/chainSimulator/chainSimulator.go	Adds a small delay before broadcasting proofs to avoid ordering drop in simulator.
integrationTests/testProcessorNode.go	Updates request handler construction with new delay argument.
integrationTests/testHeartbeatNode.go	Updates request handler construction with new delay argument.
factory/processing/processComponents.go	Passes configured proof-by-nonce delay into `NewResolverRequestHandler`.
epochStart/bootstrap/syncEpochStartMeta_test.go	Adds headers pool to epoch start syncer test args.
epochStart/bootstrap/syncEpochStartMeta.go	Wires headers pool into equivalent proofs factory during epoch start meta sync.
epochStart/bootstrap/storageProcess.go	Passes headers pool + requester delay into epoch start bootstrap components.
epochStart/bootstrap/process.go	Passes headers pool + requester delay into epoch start bootstrap components.
dataRetriever/requestHandlers/requestHandler_test.go	Updates request handler constructor calls; adds sleep to accommodate delayed nonce-proof requests.
dataRetriever/requestHandlers/requestHandler.go	Adds `requestProofByNonceDelay` and delays `RequestEquivalentProofByNonce` via goroutine + sleep.
config/config.go	Introduces `RequesterConfig.RequestProofByNonceDelayMs`.
cmd/node/config/config.toml	Adds `RequestProofByNonceDelayMs` to node config with documentation comment.

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

Copilot · 2026-03-27T14:55:03Z

+	if err != nil {
+		log.Trace("Intercepted equivalent proof with missing header, dropping it", "proof header hash", iep.proof.GetHeaderHash())
+		return err
+	}


GetHeaderByHash can legally return a (nil, nil) pair for some implementations/stubs (e.g. HeadersPoolStub), which will panic on header.GetRound(). Guard against header == nil and treat it as a missing header (return an appropriate error) before dereferencing it.

Suggested change

}

}

if header == nil {

log.Trace("Intercepted equivalent proof with missing header, dropping it", "proof header hash", iep.proof.GetHeaderHash())

return fmt.Errorf("missing header for hash %v", iep.proof.GetHeaderHash())

}

Copilot · 2026-03-27T14:55:04Z

+		HeadersPool:       &pool.HeadersPoolStub{},
 		Hasher:            &hashingMocks.HasherMock{},
 		ProofSizeChecker:  &testscommon.FieldsSizeCheckerMock{},


createMockArgInterceptedEquivalentProof uses HeadersPoolStub with default behavior (GetHeaderByHash returns (nil, nil)), but CheckValidity() now requires a real header and will dereference it. Update the stub in this helper to return a non-nil data.HeaderHandler with Round == providedRound (or make each test override GetHeaderByHashCalled) so the existing "should work" / concurrency tests don't panic.

Suggested change

HeadersPool: &pool.HeadersPoolStub{},

Hasher: &hashingMocks.HasherMock{},

ProofSizeChecker: &testscommon.FieldsSizeCheckerMock{},

HeadersPool: &pool.HeadersPoolStub{

GetHeaderByHashCalled: func(hash []byte) (data.HeaderHandler, error) {

return &block.Header{Round: providedRound}, nil

},

},

Hasher: &hashingMocks.HasherMock{},

ProofSizeChecker: &testscommon.FieldsSizeCheckerMock{},

Copilot · 2026-03-27T14:55:04Z

 func (rrh *resolverRequestHandler) RequestEquivalentProofByNonce(headerShard uint32, headerNonce uint64) {
 	epoch := rrh.getEpoch()
-	rrh.RequestEquivalentProofByNonceForEpoch(headerShard, headerNonce, epoch)
+	go func(requestEpoch uint32) {
+		time.Sleep(rrh.requestProofByNonceDelay)
+		rrh.RequestEquivalentProofByNonceForEpoch(headerShard, headerNonce, requestEpoch)
+	}(epoch)


RequestEquivalentProofByNonce now spawns a goroutine and delays the call without marking the item as requested first. Repeated calls within the delay window will schedule additional goroutines (and potentially duplicate network requests due to the non-atomic testIfRequestIsNeeded + addRequestedItems sequence). Consider registering the request key as requested before scheduling, or using a single scheduler/worker to coalesce requests per (shard,nonce).

Copilot · 2026-03-27T14:55:05Z

 		rrh.RequestEquivalentProofByNonce(shardID, requestNonce)
+		time.Sleep(time.Millisecond * 5)
 		require.True(t, wasCalled)


This test now relies on a fixed time.Sleep(5ms) to wait for the delayed goroutine in RequestEquivalentProofByNonce, which is timing-sensitive and can flake under load/slow CI. Prefer synchronizing on a channel/WaitGroup (e.g. signal from RequestDataFromNonceCalled) or using an eventual assertion with a timeout instead of a hard-coded sleep.

should be enough

Copilot · 2026-03-27T14:55:05Z

 		}

 		if !check.IfNil(pair.Proof) {
+			time.Sleep(time.Millisecond * 5) // small delay to ensure proof is not dropped as being received before header


The hard-coded time.Sleep(5ms) to ensure the proof isn't received before the header is a brittle ordering mechanism and can cause simulator flakiness on slower machines. Prefer enforcing ordering explicitly (e.g., broadcast proof only after the header is confirmed added/processed, or gate on a callback/channel) or make the delay configurable/reused from RequestProofByNonceDelayMs.

Suggested change

time.Sleep(time.Millisecond * 5) // small delay to ensure proof is not dropped as being received before header

time.Sleep(time.Duration(configs.RequestProofByNonceDelayMs) * time.Millisecond) // delay to ensure proof is not dropped as being received before header

should be ok

codecov · 2026-03-30T10:35:57Z

Codecov Report

❌ Patch coverage is 82.35294% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.54%. Comparing base (7caded5) to head (3cb5116).

Files with missing lines	Patch %	Lines
...ck/interceptedBlocks/interceptedEquivalentProof.go	55.55%	7 Missing and 1 partial ⚠️
dataRetriever/requestHandlers/requestHandler.go	92.30%	2 Missing and 1 partial ⚠️
node/chainSimulator/configs/configs.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                      @@
##           feat/supernova-async-exec    #7805      +/-   ##
=============================================================
- Coverage                      77.56%   77.54%   -0.02%     
=============================================================
  Files                            882      882              
  Lines                         123719   123751      +32     
=============================================================
+ Hits                           95958    95964       +6     
- Misses                         21402    21424      +22     
- Partials                        6359     6363       +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.

…eader

AdoAdoAdo · 2026-03-30T14:45:06Z

+		return err
+	}
+
+	if !check.IfNil(header) && header.GetRound() != iep.proof.GetHeaderRound() {


can you check also the nonce and epoch?

…eader

reject proofs received without their header + request proofs by nonce…

715d284

… delayed

sstanculeanu self-assigned this Mar 27, 2026

AdoAdoAdo requested a review from Copilot March 27, 2026 14:49

Copilot started reviewing on behalf of AdoAdoAdo March 27, 2026 14:49 View session

ssd04 reviewed Mar 27, 2026

View reviewed changes

Copilot AI reviewed Mar 27, 2026

View reviewed changes

fixed tests + fixes after copilot

6e1466c

Merge branch 'feat/supernova-async-exec' into reject-proofs-without-h…

6c70b57

…eader

AdoAdoAdo self-requested a review March 30, 2026 13:39

AdoAdoAdo reviewed Mar 30, 2026

View reviewed changes

sstanculeanu and others added 4 commits March 30, 2026 18:04

fix after review

921779d

fix after review

f018391

Merge branch 'feat/supernova-async-exec' into reject-proofs-without-h…

3cb5116

…eader

Merge branch 'feat/supernova-async-exec' into reject-proofs-without-h…

9f518bd

…eader

Base automatically changed from feat/supernova-async-exec to rc/supernova April 6, 2026 07:58

Merge branch 'rc/supernova' into reject-proofs-without-header

5d86559

sstanculeanu changed the base branch from rc/supernova to feat/testnet-fixes April 6, 2026 12:34

sstanculeanu added 2 commits April 6, 2026 16:28

reject headers once a proof already exists for the same nonce

1fefbc2

fix condition

4c7f4bb

sstanculeanu closed this Apr 7, 2026

-	}
+	}
+	if header == nil {
+		log.Trace("Intercepted equivalent proof with missing header, dropping it", "proof header hash", iep.proof.GetHeaderHash())
+		return fmt.Errorf("missing header for hash %v", iep.proof.GetHeaderHash())
+	}

	time.Sleep(time.Millisecond * 5) // small delay to ensure proof is not dropped as being received before header
	time.Sleep(time.Duration(configs.RequestProofByNonceDelayMs) * time.Millisecond) // delay to ensure proof is not dropped as being received before header

Uh oh!

Conversation

sstanculeanu commented Mar 27, 2026

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Mar 30, 2026 •

edited

Loading