Skip to content

fix(syncing): skip forced txs checks for p2p blocks #2922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
julienrbrt merged 4 commits into from
Dec 15, 2025
Merged

fix(syncing): skip forced txs checks for p2p blocks #2922

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3bd47b
fix(syncing): skip forced txs checks for p2p blocks
julienrbrt Dec 12, 2025
a40a672
doc
julienrbrt Dec 12, 2025
a2f3b5a
Merge branch 'main' into julien/fix-verify
julienrbrt Dec 12, 2025
6463cec
Merge branch 'main' into julien/fix-verify
julienrbrt Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions block/internal/syncing/syncer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ func (s *Syncer) processHeightEvent(event *common.DAHeightEvent) {
}

// only save to p2p stores if the event came from DA
if event.Source == common.SourceDA {
if event.Source == common.SourceDA { // TODO(@julienrbrt): To be reverted once DA Hints are merged (https://github.com/evstack/ev-node/pull/2891)
Copy link
Member Author

@julienrbrt julienrbrt Dec 15, 2025

Choose a reason for hiding this comment

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

turns out the todo is on the wrong condition, but we'll get it :D (should have been L594)

g, ctx := errgroup.WithContext(s.ctx)
g.Go(func() error {
// broadcast header locally only — prevents spamming the p2p network with old height notifications,
Expand Down Expand Up @@ -591,11 +591,13 @@ func (s *Syncer) trySyncNextBlock(event *common.DAHeightEvent) error {
}

// Verify forced inclusion transactions if configured
if err := s.verifyForcedInclusionTxs(currentState, data); err != nil {
s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("forced inclusion verification failed")
if errors.Is(err, errMaliciousProposer) {
s.cache.RemoveHeaderDAIncluded(headerHash)
return err
if event.Source == common.SourceDA {
if err := s.verifyForcedInclusionTxs(currentState, data); err != nil {
s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("forced inclusion verification failed")
if errors.Is(err, errMaliciousProposer) {
s.cache.RemoveHeaderDAIncluded(headerHash)
return err
}
}
Comment on lines +595 to 601
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 12, 2025

Choose a reason for hiding this comment

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

high

The current error handling for verifyForcedInclusionTxs only returns an error if it's errMaliciousProposer. Other errors, such as a transient failure to retrieve data from the DA layer, are logged but then ignored. This allows block processing to continue without proper verification, which could be problematic.

Any error from verifyForcedInclusionTxs should cause trySyncNextBlock to fail. This will allow the caller (processHeightEvent) to handle the error appropriately, such as rescheduling the event for a retry, which is the default behavior for non-critical errors. This makes the syncer more robust against transient issues.

I suggest simplifying the error handling to always return the error, which is also more consistent with how errors from validateBlock are handled just above.

		if err := s.verifyForcedInclusionTxs(currentState, data); err != nil {
			s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("forced inclusion verification failed")
			s.cache.RemoveHeaderDAIncluded(headerHash)
			return err
		}

}

Expand Down
Loading