Skip to content

fix(poll): wait_for_inclusion still has false-positive (chain 404 returns non-empty JSON error body) #27

@proofmancer

Description

@proofmancer

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

  • Replace submitter.inner().http_get(&path) with reqwest::Client::get().send() checking .status().is_success()
  • Test: ligate transfer blocks for ~12s on Mocha (real inclusion time), not 1.87s
  • Test: re-run with a high-load chain to verify polling doesn't false-positive on a 5xx mid-poll

Refs: PR #24 (info fix, correct), PR #25 (partial poll fix, has this false-positive), ligate-api#28 (sister fix in api).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions