Bug
PR #25's fix for the wait_for_inclusion URL-doubling bug is incomplete. It treats non-empty response body as "tx included", but the chain's `/v1/ledger/txs/{hash}` endpoint returns a non-empty JSON error body when the tx isn't yet indexed:
```bash
$ curl https://rpc.ligate.io/v1/ledger/txs/ltx1af6arskttyt3hzxw5rxgycsdgmv5ccw42xtxj5csxyrzfr5h5exsy0tw4c
{"status":404,"message":"Transaction 'unknown' not found","details":{"id":"unknown"}}
```
That body is non-empty, so the check if !body.trim().is_empty() => return Ok(()) matches on the very first poll and reports inclusion before the tx has actually landed. Observed during devnet smoke test: ligate transfer returned in 1.87s end-to-end, faster than real Mocha block time (~12s). The tx DID land (verified via subsequent balance query), but the cli's claim about inclusion timing was vacuous.
Root cause
The SDK's NodeClient::http_get doesn't surface HTTP status codes -- it just returns the body string. So distinguishing "200 with tx body" from "404 with JSON error body" via body content alone is brittle. PR #25 assumed empty body = 404; the chain actually returns a structured JSON error.
Fix options
A. Parse the body and check for a specific field. Tx response JSON has a tx_hash field (or similar). Error JSON has status + message. Match on the presence of tx_hash. Brittle if the response shape changes.
B. Use reqwest directly in wait_for_inclusion_via_http. Bypass the SDK's http_get entirely. reqwest::get(url).status().is_success() gives us proper 2xx/4xx discrimination. Mirrors what query.rs already does.
C. Patch the SDK's http_get. Have it return Err on non-2xx, restoring the behaviour the old code comment claimed existed. Upstream PR to sovereign-sdk-fork.
Recommended
Option B, same shape as query.rs (which doesn't use the SDK's http_get for similar reasons). Smallest scope, no upstream dependency, deterministic.
Same bug in api
Sister fix in ligate-api#28's wait_for_inclusion has the same shape. Fix the api alongside.
Test plan
Refs: PR #24 (info fix, correct), PR #25 (partial poll fix, has this false-positive), ligate-api#28 (sister fix in api).
Bug
PR #25's fix for the
wait_for_inclusionURL-doubling bug is incomplete. It treats non-empty response body as "tx included", but the chain's `/v1/ledger/txs/{hash}` endpoint returns a non-empty JSON error body when the tx isn't yet indexed:```bash
$ curl https://rpc.ligate.io/v1/ledger/txs/ltx1af6arskttyt3hzxw5rxgycsdgmv5ccw42xtxj5csxyrzfr5h5exsy0tw4c
{"status":404,"message":"Transaction 'unknown' not found","details":{"id":"unknown"}}
```
That body is non-empty, so the check
if !body.trim().is_empty() => return Ok(())matches on the very first poll and reports inclusion before the tx has actually landed. Observed during devnet smoke test:ligate transferreturned in 1.87s end-to-end, faster than real Mocha block time (~12s). The tx DID land (verified via subsequent balance query), but the cli's claim about inclusion timing was vacuous.Root cause
The SDK's
NodeClient::http_getdoesn't surface HTTP status codes -- it just returns the body string. So distinguishing "200 with tx body" from "404 with JSON error body" via body content alone is brittle. PR #25 assumed empty body = 404; the chain actually returns a structured JSON error.Fix options
A. Parse the body and check for a specific field. Tx response JSON has a
tx_hashfield (or similar). Error JSON hasstatus+message. Match on the presence oftx_hash. Brittle if the response shape changes.B. Use reqwest directly in
wait_for_inclusion_via_http. Bypass the SDK'shttp_getentirely.reqwest::get(url).status().is_success()gives us proper 2xx/4xx discrimination. Mirrors whatquery.rsalready does.C. Patch the SDK's
http_get. Have it returnErron non-2xx, restoring the behaviour the old code comment claimed existed. Upstream PR to sovereign-sdk-fork.Recommended
Option B, same shape as
query.rs(which doesn't use the SDK's http_get for similar reasons). Smallest scope, no upstream dependency, deterministic.Same bug in api
Sister fix in ligate-api#28's wait_for_inclusion has the same shape. Fix the api alongside.
Test plan
submitter.inner().http_get(&path)withreqwest::Client::get().send()checking.status().is_success()ligate transferblocks for ~12s on Mocha (real inclusion time), not 1.87sRefs: PR #24 (info fix, correct), PR #25 (partial poll fix, has this false-positive), ligate-api#28 (sister fix in api).