Skip to content

Fix: SingleInstructionPipeline early return prevents processing subsequent instructions #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix: SingleInstructionPipeline early return prevents processing subsequent instructions #141

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions crates/runtime/src/instruction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl SingleInstructionPipeline {
/// # Errors
/// Returns an error if the inner pipeline fails.
pub async fn handle(&self, txn: &TransactionUpdate) -> Result<(), PipelineErrors> {
let mut err = None;
let ixs = InstructionUpdate::parse_from_txn(txn).map_err(PipelineErrors::parse)?;
let pipe = &self.0;

Expand All @@ -107,15 +108,15 @@ impl SingleInstructionPipeline {
match res {
Ok(()) => (),
Err(PipelineErrors::AlreadyHandled(h)) => h.as_unit(),
Err(e) => {
let handled = e.handle::<InstructionUpdate>(&pipe.id());

return Err(PipelineErrors::AlreadyHandled(handled));
},
Comment on lines -110 to -114
Copy link
Collaborator

@kespinola kespinola Oct 5, 2025

Choose a reason for hiding this comment

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

If there is any instruction error then the whole transaction is aborted. This error handling is designed to mimic the same transaction handling as the svm runtime. Is this not the cases?

Copy link
Author

@a00012025 a00012025 Oct 5, 2025

Choose a reason for hiding this comment

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

I think it happens when one handler returned an error when parsing an instruction, even that instruction succeeds. I notice this issue because it didn't parse the second jupiter route instruction in this tx: https://solscan.io/tx/3YEYmqJeRJHDNX4B7yY5mmjeGbT5scMw977LXQfC4CBTihX7qoSW3AQiZnFD8Us5jz2LXoTSu5HPF5XrfqzBcWUZ

Copy link
Author

@a00012025 a00012025 Oct 6, 2025

Choose a reason for hiding this comment

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

It's because I returned yellowstone_vixen_core::ParseError::Filtered in some of my parsers which will be treated as error here

Copy link
Collaborator

@kespinola kespinola Oct 21, 2025

Choose a reason for hiding this comment

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

@a00012025 I haven't gotten back to this but I am hesitant. What if you handle the error in your parser and don't return an error unless its something you want to fail?

I do feel as an error in parsing any instruction in the transaction should early exit but lets keep talking about it.

Err(e) => err = Some(e.handle::<InstructionUpdate>(&pipe.id())),
}
}

Ok(())
if let Some(h) = err {
Err(PipelineErrors::AlreadyHandled(h))
} else {
Ok(())
}
}
}

Expand Down