Skip to content

Conversation

@Fallengirl
Copy link

Description

snapshotManager.SnapshotIfApplicable() was called without nil check in Commit(), while all other usages in the same file properly check for nil first. This would cause a panic if snapshot manager wasn't configured.

Copy link
Contributor

@aljo242 aljo242 left a comment

Choose a reason for hiding this comment

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

we require tests for all functionality changes

@Fallengirl
Copy link
Author

Added test TestABCI_Commit_WithNilSnapshotManager. Note: SnapshotIfApplicable already has an internal nil check, so this change is technically safe either way. However, this explicit nil check follows the same defensive pattern used elsewhere in Commit() — see Precommiter check on line 1019 and PrepareCheckStater check on line 1055. It makes the nil-safety explicit at the call site rather than relying on the internal implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants