Add defensive coding and matching unit tests for fee bumps#605
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds defensive nil pointer checks when extracting Soroban transaction data from transaction envelopes, preventing panics when processing fee-bump transactions containing Soroban operations. The changes refactor the code into well-structured helper functions and add comprehensive test coverage.
Changes:
- Introduced
sorobanDataFromEnvelopeandsorobanDataFromTxhelper functions with defensive nil checks for all pointer accesses - Refactored existing code to use new helper functions, eliminating unsafe nil pointer access in fee-bump transaction handling
- Added unit test verifying that fee-bump transactions with missing Soroban data return appropriate error messages instead of panicking
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/stellar-rpc/internal/methods/simulate_transaction.go | Added two helper functions with comprehensive nil checks; replaced unsafe direct field access with calls to these helpers |
| cmd/stellar-rpc/internal/methods/simulate_transaction_test.go | Added test case and helper function to verify fee-bump transactions without Soroban data are handled gracefully |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
karthikiyer56
approved these changes
Feb 19, 2026
Shaptic
commented
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Add guardrails around accessing possibly-
nilpointers when inspecting transactions.Why
When passing a fee-bump transaction containing a soroban operation, the following check:
would cause a
nilpointer reference becauseV1isnil. While this has no security implications (the panic is simply caught and logged like any other error), it is good to be defensive.