Skip to content

instruction: Forward compatibility for BorshIoError serde#410

Merged
joncinque merged 4 commits intoanza-xyz:maintenance/v2.xfrom
joncinque:ixerrv2
Oct 29, 2025
Merged

instruction: Forward compatibility for BorshIoError serde#410
joncinque merged 4 commits intoanza-xyz:maintenance/v2.xfrom
joncinque:ixerrv2

Conversation

@joncinque
Copy link
Copy Markdown
Collaborator

Problem

As noted in #404, InstructionError in v2 can't deserialize the new BorshIoError variant in v3 because we removed the string component.

Summary of changes

After trying many different things, this seems like the least bad option, and actually recommended by serde's maintainer for this kind of situation.

Agave should not be updated to use this, but all clients can pull in the new crate for their v2 builds to work with v3 RPC nodes.

cc @fanatid

#### Problem

As noted in anza-xyz#404, InstructionError in v2 can't deserialize the new
BorshIoError variant in v3 because we removed the string component.

#### Summary of changes

After trying many different things, this seems like the least bad
option, and actually recommended by serde's maintainer for this kind of
situation.

Agave should not be updated to use this, but all clients can pull in the
new crate for their v2 builds to work with v3 RPC nodes.
Copy link
Copy Markdown
Contributor

@febo febo left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Copy Markdown
Contributor

@fanatid fanatid left a comment

Choose a reason for hiding this comment

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

lgtm

Comment thread instruction/src/error.rs
{
let s = serde_json::Value::deserialize(deserializer)?;
if s.as_str().is_some_and(|v| v == "BorshIoError") {
Ok(Self::BorshIoError(String::new()))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had seen that default value is Unknown, should it be from default? but probably empty is valid 😕

https://github.com/anza-xyz/solana-sdk/blob/transaction-error%40v2.2.1/instruction/src/error.rs#L410

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I thought about that, but decided to go with the simplest option, since I don't even think RPC returns a value for these errors when coming from blocks.

@joncinque joncinque merged commit 5b2614e into anza-xyz:maintenance/v2.x Oct 29, 2025
25 checks passed
@joncinque joncinque deleted the ixerrv2 branch October 29, 2025 19:15
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.

3 participants