fix(block): validate block after applying instead of before during sync

[julienrbrt]

Overview

We validate after during execution, but before during syncing, but the signature verifier provided by ev-abci needs some data that is saved after applying.

[gemini-code-assist]

Summary of Changes

Hello @julienrbrt, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request implements a fix that alters the sequence of operations during block synchronization. Specifically, it changes when a block is validated: instead of validating a block before it's applied to the blockchain state, the validation now occurs after the block has been applied. This adjustment allows the validation process to leverage the updated state of the blockchain, which can be critical for certain verification functions.

Highlights

• Block Validation Timing Change: The logic for validating incoming blocks during the synchronization process has been reordered. Previously, blocks were validated before being applied to the blockchain state. Now, the validation occurs after the block has been applied.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[github-actions]

The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Aug 19, 2025, 3:25 PM

[gemini-code-assist]

Code Review

This pull request modifies the block synchronization logic to perform block validation after applying the block, rather than before. The stated reason is to support custom verification functions that may depend on the state of the blockchain after transaction execution. This change aligns the synchronization path with the block production path.

My review focuses on the trade-offs of this change. While necessary for the described use case, it introduces the possibility of performing expensive work in applyBlock that is then discarded if validation fails. I've added a comment to discuss this and suggest considering a more transactional approach for block application in the future to improve robustness.

[gemini-code-assist]

By moving validation to after applyBlock, we risk performing computationally expensive work that will be discarded if validation subsequently fails. If applyBlock (specifically, m.exec.ExecuteTxs) has side effects on the executor's in-memory state, a validation failure would leave the executor in an intermediate state, which would likely only be resolved by a node restart.

While this change is necessary if signature verification depends on transaction execution results, it's worth acknowledging this trade-off. For future improvements, consider if the Executor interface could be extended to support a more transactional block application process. For example:

1. PrepareBlock to execute transactions and return a handle to a candidate state.
2. Validate the block against this candidate state.
3. CommitBlock or AbortBlock based on the validation outcome.

This would create a more robust and explicit flow, avoiding wasted work and transient inconsistencies.

[claude]

Claude finished @julienrbrt's task —— View job

---

I'll review this pull request and provide feedback on code quality, potential issues, and other considerations.

Todo List

• Read the modified block/sync.go file to understand the changes
• Look for related test files and assess test coverage
• Analyze code quality and potential issues
• Check for performance and security implications
• Provide comprehensive review feedback

Code Review Summary

This PR changes the block validation timing during synchronization from before applying the block to after applying the block. The change is necessary to support signature verifiers in ev-abci that depend on post-execution state.

✅ Positive Aspects

1. Clear Intent: The change aligns sync path with block production path, which validates after applying the block (lines 152-161 in sync.go vs manager.go:641).

2. Good Test Coverage: The existing test suite in block/sync_test.go has comprehensive coverage of sync scenarios and should catch any regressions from this change. Tests validate the complete flow including validation.

3. Proper Error Handling: Error propagation is maintained correctly - if validation fails after applying, the error bubbles up appropriately.

4. Consistent Comments: The code includes helpful comments explaining the rationale (lines 157-158).

⚠️ Concerns & Issues

Performance Impact

High Priority: Moving validation after applyBlock creates a significant performance inefficiency. If validation fails:

• Expensive transaction execution work (m.exec.ExecuteTxs) is wasted
• Potential state mutations in the executor that need cleanup
• Resources spent on block application that won't be committed

Reference: Lines 152-161 in block/sync.go:trySyncNextBlock()

State Consistency Risk

Medium Priority: If applyBlock modifies executor in-memory state and validation fails, the executor may be left in an inconsistent state. Unlike the block production path, sync doesn't have transactional semantics.

Error Recovery

Medium Priority: The current error handling doesn't address cleanup after failed validation. If validation fails, the manager state remains unchanged, but executor state may be corrupted.

🔧 Recommendations

1. Consider Transactional Approach: Future enhancement could implement a transactional block application pattern:

   // Pseudo-code
   candidate := m.exec.PrepareBlock(ctx, txs, ...)
   if err := m.Validate(ctx, header, data, candidate); err != nil {
       return err  // No expensive work wasted
   }
   newState := m.exec.CommitBlock(candidate)

2. Add Recovery Logic: Consider adding explicit state recovery if validation fails after execution.

3. Monitor Performance: Track metrics for validation failures to quantify the performance impact in production.

4. Documentation: Consider updating architecture docs to explain the trade-offs made for supporting post-execution validation.

🧪 Test Coverage Assessment

Good: Existing tests cover the modified flow comprehensively:

• Single block processing (both header/data orderings)
• Multiple block scenarios
• Out-of-order arrival
• Duplicate handling
• Error conditions

The tests should catch any behavioral changes from this modification.

🔒 Security Considerations

No major concerns: The change doesn't introduce security vulnerabilities. Validation still occurs and errors are properly handled.

📝 Code Quality

Good: The implementation follows existing patterns and maintains code consistency. The change is minimal and focused.

Conclusion

This is a necessary architectural change that introduces a performance trade-off to enable ev-abci signature verification. While not ideal from a performance perspective, it's a pragmatic solution. The excellent test coverage and clear implementation minimize risk.

Recommendation: Approve with awareness of the performance implications and potential for future optimization.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.74%. Comparing base (a7dbf77) to head (815a141).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
block/sync.go	0.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2579      +/-   ##
==========================================
+ Coverage   72.65%   72.74%   +0.09%     
==========================================
  Files          72       72              
  Lines        7311     7310       -1     
==========================================
+ Hits         5312     5318       +6     
+ Misses       1574     1568       -6     
+ Partials      425      424       -1     

Flag	Coverage Δ
combined	72.74% <0.00%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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