Repository navigation
fix(stellar): read and validate Horizon strict-receive responses - #251
Merged
truthixify merged 1 commit intoOct 16, 2026
Merged
truthixify merged 1 commit into
truthixify merged 1 commit into
Conversation
findStrictReceivePath expected a bare array, but Horizon answers with a _embedded.records envelope, and it sent destination_asset plus a dummy source_account, which Horizon rejects with 400. Every live call failed. - send source_assets and destination_asset_type/_code/_issuer - read _embedded.records; the endpoint is not paginated - validate every record: amounts, asset type/code/issuer, that source, destination and destination_amount match the request, and at most 5 path hops; reject more than 100 records - read the body as a stream and stop past 256 KiB; cap error bodies too - use the Horizon client's default connect/request timeouts - validate destAmount before the request; strip a trailing slash - return the record with the smallest source_amount - add HorizonResponseError for malformed or oversized responses and throw RPCRequestError for non-2xx statuses Closes wraith-protocol#236
|
@izzetemredemir Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
truthixify
approved these changes
Oct 16, 2026
truthixify
left a comment
Contributor
There was a problem hiding this comment.
The strict-receive parser now matches Horizon's envelope and request shape, validates assets, amounts and path bounds, caps both success and error bodies, and applies the shared deadline across headers and streaming reads. I reran all 47 focused tests plus build, API extraction, and bundle-size checks on the merge ref; all pass.
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
Reads the real Horizon envelope.
findStrictReceivePathcast the response to a bare array, but Horizon answers{ "_embedded": { "records": [...] } }. Records are now read from_embedded.records. The endpoint is not paginated (the response has no_links/cursor), so all records come in one page; this is documented on the function and indocs/stellar-path-payment.md.Fixes the request too. While testing against horizon-testnet I found every live call failed with HTTP 400 before parsing:
destination_asset=nativeis not a Horizon parameter (invalid_field: destination_asset_type);source_accountfails the strkey checksum;source_accountandsource_assetstogether.The request now sends
source_assetsplusdestination_asset_type/destination_asset_code/destination_asset_issuer. This goes a little beyond the issue title, but without it the parsing fix can't be exercised against a real server.Validates every record before use:
source_amountanddestination_amountare positive Stellar amounts (≤ 7 decimals, within int64 stroops);destination_amountequalsdestAmount; source and destination assets are exactlysendAssetandreceiveAsset; every asset's type matches its code and has a valid issuer;pathis an array of at most 5 assets (Asset path<5>in the XDR). One bad record rejects the whole response.Rejects oversized payloads without buffering them: the body is read as a stream and abandoned (stream cancelled) once it passes 256 KiB; a declared
Content-Lengthover the limit is rejected before reading; more than 100 records is rejected. Error bodies are also read only up to 2 KiB, and a failure while reading one still surfaces asRPCRequestErrorwith the status. Without a body stream (some fetch polyfills) the limit is still checked in bytes.Timeouts: the request goes through the same
AttemptDeadlineas the SDK's Horizon client (10 s for headers, 30 s total,RPCTimeoutError), so a server that never answers or trickles the body can't hang the call. No retries are added; the defaults are not configurable here, to keep the options unchanged.Input checks:
destAmountmust be a positive amount with at most 7 decimals and is checked before the request, so thedestination_amountcomparison always applies. Trailing slashes onhorizonUrlare stripped, ascreateHorizonClientdoes.Typed errors: new
HorizonResponseError(WRAITH/NETWORK/HORIZON_RESPONSE, aWraithNetworkErrorwithurl,field,reason;error.fieldnames the rejected value, e.g._embedded.records[0].source_amount). A non-2xx status now throws the existingRPCRequestErrorinstead of a plainError. "No payment path found" stays a plainErrorwith the same message. Untrusted values are truncated in messages.Picks the cheapest valid record (smallest
source_amount) instead of trusting the server's order.HorizonResponseErroris exported from the package root, soetc/sdk.api.mdis regenerated; it is added todocs/errors.md, and there is a### Fixedentry in the CHANGELOG.Design choices you may want differently: one invalid record rejects the whole response rather than being skipped (a server sending malformed paths is not trusted for the others), and there is no retry on 429/5xx, as before; I can route this through
createHorizonClientwith a size-capped read in a follow-up if you prefer.Behaviour changes for callers: errors thrown on HTTP failures are now
RPCRequestError(previouslyError('Horizon path finding failed: …')), malformed responses throwHorizonResponseError, slow responses throwRPCTimeoutError, and an invaliddestAmountthrows before any request. Successful results keep the same shape. "No payment path found" is still a plainErrorwith the same message; happy to give it a typed error too if you prefer.Validation
Hostile fixtures in
test/chains/stellar/path-payment.test.ts(22 table cases, plus an endless body withoutContent-Lengththat must be cancelled after ~256 KiB, an oversizedContent-Lengthwith an endless body, an endless 500 error body, a 503 whose body fails to arrive, a no-stream response over the byte limit, a huge field value, a server that never answers, and invaliddestAmountvalues): non-JSON body, bare array, missing_embedded, non-array records, 101 records, non-object record, wrong source / destination asset, asset type that doesn't fit the code, native asset carrying an issuer, unknown asset type, negative / zero / exponent / 8-decimal / over-int64 / numeric amounts, destination amount mismatch, non-array path, 6-hop path, non-object hop, hop with an invalid issuer. The old mocks returned a bare array, which is why the tests never caught the envelope bug; they now use realResponseobjects with Horizon's shape.Negative check: run against the code on
develop, 39 of the 47 tests inpath-payment.test.tsfail (the 8 that pass are the unchangedbuildPathStealthPaymenttests).Live check against horizon-testnet (built
dist/):The Stellar bundle is now close to its size limit (about 0.3 kB left in ESM); flagging it in case you'd rather raise the limit or trim elsewhere.
Closes #236