Skip to content

[WIP] Backport InstructionError parsing fixes#8581

Closed
BriungRi wants to merge 2 commits intoanza-xyz:v2.3from
BriungRi:bl/v2.3.14
Closed

[WIP] Backport InstructionError parsing fixes#8581
BriungRi wants to merge 2 commits intoanza-xyz:v2.3from
BriungRi:bl/v2.3.14

Conversation

@BriungRi
Copy link
Copy Markdown

Problem

Summary of Changes

Fixes #

@BriungRi BriungRi changed the title Backport e72266ca6f [WIP] Backport e72266ca6f Oct 20, 2025
@mergify
Copy link
Copy Markdown

mergify Bot commented Oct 20, 2025

If this PR represents a change to the public RPC API:

  1. Make sure it includes a complementary update to rpc-client/ (example)
  2. Open a follow-up PR to update the JavaScript client @solana/kit (example)

Thank you for keeping the RPC clients in sync with the server API @BriungRi.

@mergify mergify Bot requested a review from a team October 20, 2025 21:52
* Add `UiTransactionError` and `UiTransactionResult`

* Fixup callsites and tests

* Review feedback

* Changed CLI types to use `UiTransactionError`
* Eliminated `UiTransactionResult` in favour of `Result<(), UiTransactionError>`
* Implemented `std::error::Error` on `UiTransactionError` which just forwards to `TransactionError`
* Implemented `fmt::Display` on `UiTransactionError` which just forwards to `TransactionError`
* Made the inner error private on `UiTransactionError`
* Eliminated `Deref` implementation, requiring explicit conversion

* * Results, all the way down
* Make the `InstructionError` deserialize actually work by taking all of the bytes

* Remove stray dependency

* Fix the deserializer
@BriungRi BriungRi changed the title [WIP] Backport e72266ca6f [WIP] Backport InstructionError parsing fixes Oct 20, 2025
@BriungRi BriungRi force-pushed the bl/v2.3.14 branch 2 times, most recently from d3883e8 to 178d549 Compare October 20, 2025 22:36
Enhance the InstructionError deserializer within UiTransactionError to handle
both unit variant and newtype variant serialization formats. This improves
compatibility when deserializing errors from different SDK versions.

Specifically handles the case where BorshIoError may be serialized as:
- Unit variant: "BorshIoError" (SDK v2.0+)
- Newtype variant: {"BorshIoError": "message"} (older SDKs)

The deserializer now tries the direct deserialization first (handles unit
variants), and falls back to converting the format if needed (handles newtype
variants in older SDKs).

This defensive coding ensures forward and backward compatibility across SDK
version boundaries.
@steviez
Copy link
Copy Markdown

steviez commented Oct 20, 2025

@BriungRi - Can you comment on why you opened another PR against v2.3 given @bw-solana's comment from #8579 (comment) ?

@BriungRi
Copy link
Copy Markdown
Author

@BriungRi - Can you comment on why you opened another PR against v2.3 given @bw-solana's comment from #8579 (comment) ?

Setting up a PR on GH lets me visually validate the two commits I've selected are valid backports from master to 2.3. If this were my own repo, it also lets me run CI against these commits too. FWIW, this is a draft PR and not meant for review.

@steviez
Copy link
Copy Markdown

steviez commented Oct 21, 2025

FWIW, this is a draft PR and not meant for review.

Since you're not an Anza member, several people get notifications when you open a PR or update it (push, comment, rename, etc). And even more people for this commit since it is against the v2.3 (stable) branch which has extra branch protections.

If you have a noisy PR, people are likely to unsubscribe from it which could result in your PR later getting ignored if/when you do actually want a review.

Setting up a PR on GH lets me visually validate the two commits

I see. If you just want the diff, you can do git diff HEAD~X HEAD > my_diff.patch. If you want the diff in GitHub specifically, you could also open a PR in your own fork

are valid backports from master to 2.3.

If you're commenting on whether they apply cleanly, you can do:

$ git checkout test-v2.3 v2.3
$ git cherry-pick YOUR_HASH

The CLI will yell at you if there is a merge conflict

If this were my own repo, it also lets me run CI against these commits too

Yeah, we intentionally limit who gets CI run automatically since we've unfortunately had abuse in the past.

@t-nelson t-nelson closed this Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants