Skip to content

Comments

fix: add nil guards for Solana tx metadata and safe signature parsing#4550

Open
ws4charlie wants to merge 2 commits intomainfrom
fix/solana-nil-meta-and-must-signature
Open

fix: add nil guards for Solana tx metadata and safe signature parsing#4550
ws4charlie wants to merge 2 commits intomainfrom
fix/solana-nil-meta-and-must-signature

Conversation

@ws4charlie
Copy link
Contributor

@ws4charlie ws4charlie commented Feb 24, 2026

Summary

  • Add Meta != nil guard before accessing txResult.Meta.Err in three locations to prevent potential nil pointer dereference, consistent with the existing pattern at inbound_parser.go:217
  • Replace MustSignatureFromBase58 (panics on invalid input) with SignatureFromBase58 (returns error) in inbound_tracker.go, consistent with the outbound path at outbound.go:234

Changes

File Fix
observer/outbound.go:251 Meta != nil guard in CheckFinalizedTx
signer/outbound_tracker_reporter.go:73 Meta != nil guard in reportToOutboundTracker
observer/inbound.go:281 Meta != nil guard in FilterInboundEvents
observer/inbound_tracker.go:54 Safe SignatureFromBase58 + error handling

Test plan

  • go build ./zetaclient/chains/solana/... compiles clean
  • go test ./zetaclient/chains/solana/observer/ passes
  • go test ./zetaclient/chains/solana/signer/ passes
  • CI passes

🤖 Generated with Claude Code


Note

Low Risk
Defensive nil/error handling changes only; behavior changes are limited to skipping malformed tracker entries and avoiding potential nil dereferences.

Overview
Prevents Solana observer/signer crashes by adding Meta != nil checks before reading Meta.Err when filtering inbound events, validating finalized outbound txs, and monitoring outbound confirmations.

Inbound tracker processing now uses SignatureFromBase58 with error handling instead of MustSignatureFromBase58, logging and skipping trackers with invalid tx hashes rather than panicking.

Written by Cursor Bugbot for commit 4e25f05. Configure here.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Added nil safety guards for Solana transaction metadata to prevent crashes during observation and signing
    • Improved signature parsing with better error handling that gracefully skips invalid signatures instead of panicking
    • Enhanced overall robustness in Solana transaction processing

- Add nil check for txResult.Meta before accessing Meta.Err in three
  locations (observer/outbound.go, signer/outbound_tracker_reporter.go,
  observer/inbound.go) to prevent potential nil pointer dereference
- Replace MustSignatureFromBase58 with SignatureFromBase58 in
  inbound_tracker.go to gracefully handle invalid base58 tx hashes
  instead of panicking

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50aa497 and 6afe9f7.

📒 Files selected for processing (5)
  • changelog.md
  • zetaclient/chains/solana/observer/inbound.go
  • zetaclient/chains/solana/observer/inbound_tracker.go
  • zetaclient/chains/solana/observer/outbound.go
  • zetaclient/chains/solana/signer/outbound_tracker_reporter.go

📝 Walkthrough

Walkthrough

The pull request enhances robustness of Solana transaction processing by introducing defensive nil checks for transaction metadata fields and replacing unsafe signature parsing with error-handling alternatives to prevent nil pointer dereferences and panics across transaction tracking and validation.

Changes

Cohort / File(s) Summary
Documentation
changelog.md
Added entry documenting nil guard and safe signature parsing improvements.
Observer Transaction Metadata Validation
zetaclient/chains/solana/observer/inbound.go, zetaclient/chains/solana/observer/outbound.go
Added nil guards before accessing txResult.Meta.Err to prevent nil pointer dereferences when evaluating transaction failure status.
Observer Signature Parsing Safety
zetaclient/chains/solana/observer/inbound_tracker.go
Replaced solana.MustSignatureFromBase58 with solana.SignatureFromBase58, introducing error handling to gracefully skip invalid transaction hashes and prevent panics.
Signer Transaction Metadata Validation
zetaclient/chains/solana/signer/outbound_tracker_reporter.go
Added nil guard for tx.Meta before checking tx.Meta.Err to prevent nil pointer access during transaction failure determination.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main changes: adding nil guards for Solana transaction metadata and implementing safe signature parsing.
Description check ✅ Passed The description provides a clear summary, detailed change table, comprehensive test results, and risk assessment. However, the template-required testing checkboxes are incomplete.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/solana-nil-meta-and-must-signature

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.16.1)
zetaclient/chains/solana/observer/inbound_tracker.go

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

zetaclient/chains/solana/observer/inbound.go

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

zetaclient/chains/solana/observer/outbound.go

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

  • 1 others

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ws4charlie ws4charlie added chain:solana SOLANA_TESTS Run make start-solana-test labels Feb 24, 2026
@ws4charlie ws4charlie requested review from a team and morde08 February 24, 2026 17:53
ws4charlie added a commit that referenced this pull request Feb 24, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ws4charlie ws4charlie force-pushed the fix/solana-nil-meta-and-must-signature branch from e68f1d9 to 6afe9f7 Compare February 24, 2026 17:55
@ws4charlie ws4charlie marked this pull request as ready for review February 24, 2026 17:56
@ws4charlie ws4charlie requested a review from a team as a code owner February 24, 2026 17:56
@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...taclient/chains/solana/observer/inbound_tracker.go 0.00% 4 Missing ⚠️
.../chains/solana/signer/outbound_tracker_reporter.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

Labels

chain:solana SOLANA_TESTS Run make start-solana-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants