fix(api-server): surface RPC events on simulate response (#1162) - #1238
Open
gebibd00-jpg wants to merge 1 commit into
Open
fix(api-server): surface RPC events on simulate response (#1162)#1238gebibd00-jpg wants to merge 1 commit into
gebibd00-jpg wants to merge 1 commit into
Conversation
…#1162) `SimulateTransactionResult.events` was being parsed by the Soroban `simulateTransaction` deserializer but then dropped on the floor by the simulate handler - confirmed by the `#[allow(dead_code)]` attribute and a repo-wide grep showing no other reference to the field. The RPC's `events` array carries diagnostic/contract events (emitted events, failure reasons) that are useful context for callers trying to understand a simulation result, especially one that reports `success: false`. Fix: - `FeeBreakdown` gains an `events: Vec<serde_json::Value>` field with a doc comment explaining the contract: present but possibly empty on the heuristic fallback path. - `simulate()` propagates `result.events` into the `FeeBreakdown` on the live-RPC success path; the heuristic fallback path initializes `events` to an empty vec. - `SimulationDetail` in types.rs gains a matching `events: Vec<serde_json::Value>` field with `#[serde(default)]` so existing clients deserializing older responses without the field still parse cleanly. - `handlers::simulate` populates `SimulationDetail.events` from the breakdown. Tests (both stand up an in-process axum mock via `tokio::net::TcpListener::bind("127.0.0.1:0")` so they don't depend on any fixed port or a live RPC): - `test_simulate_surfaces_rpc_events_in_simulation_detail` - mock returns two events (a contract Transfer and a diagnostic message); asserts the API surfaces both in `simulation.events`. - `test_simulate_events_empty_on_heuristic_fallback` - uses the existing `test_app()` (RPC unreachable) and asserts `simulation.events` is present but empty. I do not have a local Rust toolchain, so I could not run `cargo test` myself - please run the test suite before merging. The diff is mechanical: an `events` field threaded through four files plus two self-contained tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SimulateTransactionResult.eventswas being parsed by the SorobansimulateTransactiondeserializer but then dropped on the floor by the simulate handler — confirmed by the#[allow(dead_code)]attribute and a repo-wide grep showing no other reference to the field. The RPCseventsarray carries diagnostic/contract events (emitted events, failure reasons) that are useful context for callers trying to understand a simulation result, especially one that reportssuccess: false.Fix
FeeBreakdowngains anevents: Vec<serde_json::Value>field with a doc comment explaining the contract: present but possibly empty on the heuristic fallback path.simulate()propagatesresult.eventsinto theFeeBreakdownon the live-RPC success path; the heuristic fallback path initializeseventsto an empty vec.SimulationDetailin types.rs gains a matchingevents: Vec<serde_json::Value>field with#[serde(default)]so existing clients deserializing older responses without the field still parse cleanly.handlers::simulatepopulatesSimulationDetail.eventsfrom the breakdown.Tests
Both tests stand up an in-process axum mock via
tokio::net::TcpListener::bind("127.0.0.1:0")so they do not depend on any fixed port or a live RPC:test_simulate_surfaces_rpc_events_in_simulation_detail— mock returns two events (a contractTransferand a diagnostic message); asserts the API surfaces both insimulation.events.test_simulate_events_empty_on_heuristic_fallback— uses the existingtest_app()(RPC unreachable) and assertssimulation.eventsis present but empty.I do not have a local Rust toolchain, so I could not run
cargo testmyself — please run the test suite before merging. The diff is mechanical: aneventsfield threaded through four files plus two self-contained tests.Closes
#1162