Skip to content

fix(client): prevent panic when parsing events from multi-instruction txs#4341

Open
alexchenai wants to merge 2 commits intosolana-foundation:masterfrom
alexchenai:fix/events-multi-instruction-panic
Open

fix(client): prevent panic when parsing events from multi-instruction txs#4341
alexchenai wants to merge 2 commits intosolana-foundation:masterfrom
alexchenai:fix/events-multi-instruction-panic

Conversation

@alexchenai
Copy link
Copy Markdown

Summary

Fix panic in anchor-client when parsing events from transactions containing multiple top-level instructions.

Issue: #1941

Root Cause

The Execution struct's program() and pop() methods use assert!(!self.stack.is_empty()) which panics when the execution stack is depleted. This happens when a transaction contains multiple instructions - after the first instruction completes, its success log causes a pop() that empties the stack. The next iteration then calls program() on the empty stack, triggering the panic.

Fix

  • Execution::program() now returns Option instead of panicking on empty stack
  • Execution::pop() silently handles empty stack (no-op) instead of panicking
  • parse_logs_response updated to compare against Option via as_deref()
  • When the stack is empty, log lines are correctly processed as system logs

Tests

  • test_parse_logs_response_multiple_instructions: Regression test with 3 interleaved top-level instructions
  • test_execution_pop_empty_stack_no_panic: Unit test verifying graceful empty stack handling

… transactions

The `Execution` struct's `program()` and `pop()` methods used assertions
that would panic when the stack was empty. This occurs when a transaction
contains multiple top-level instructions, because the stack is emptied
after processing each instruction's "success" log.

Replace the panicking assertions with defensive handling:
- `program()` now returns `Option<String>` instead of panicking
- `pop()` silently handles empty stack instead of panicking
- Update `parse_logs_response` to handle the `Option` return type

Fixes coral-xyz#1941

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 19, 2026

@alexchenai is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@jamie-osec
Copy link
Copy Markdown
Collaborator

This appears to simply ignore the errors rather than correctly handling the logic. Please refer to previous attempts to fix this: #1954

…ctions

The previous implementation changed program() to return Option<String>
and silently ignored pop() on empty stacks. This was correctly noted as
'simply ignoring the errors rather than correctly handling the logic.'

The real fix: introduce pop_or_replace(next_program: Option<String>) that
atomically replaces the stack top when transitioning to a new top-level
[1] instruction, or just pops for CPI returns. This keeps the stack
invariant intact — it is never empty between sequential top-level
instructions — so program() can safely return String and panic on
genuinely impossible empty stacks.

Reverences: solana-foundation#1954 (original approach), solana-foundation#1941 (root issue)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alexchenai
Copy link
Copy Markdown
Author

Thanks for the feedback and pointing to #1954. You were right — returning Option<String> and silently popping an empty stack was just ignoring the problem rather than fixing it.

Updated the fix to use pop_or_replace(next_program: Option<String>) which atomically transitions from one top-level instruction to the next, keeping the stack invariant intact. program() is back to returning String and will panic on a genuinely empty stack (which should now be unreachable).

The key insight from #1954: when we see Program X success followed by Program Y invoke [1], we should replace X with Y rather than pop-then-push, so the stack is never momentarily empty.

alexchenai added a commit to alexchenai/anchor that referenced this pull request Mar 24, 2026
…tead of ignoring them

Per maintainer feedback on PR solana-foundation#4341: the previous implementation either
panicked (expect()) or silently discarded errors (Option::and_then chaining).

Changes:
- program() now returns Result<String, ClientError> and propagates an
  empty-stack condition as LogParseError rather than panicking
- pop_or_replace() now returns Result<(), ClientError> and propagates
  an empty-stack pop as LogParseError rather than silently succeeding
- The invoke [1] log parsing now uses ? to propagate a regex failure
  as LogParseError rather than silently treating it as None

A malformed invoke log or empty stack is an error condition that should
surface to the caller, not be swallowed.
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