fix(sdk): surface ARC broadcast errors as Err and parse cached tx bytes in get_transaction

[E-Jacko]

Summary

Two-part fix to packages/runar-rs/src/sdk/wallet.rs:

1. R1 — WalletProvider::broadcast returns Err(...) on ARC HTTP non-2xx and on network failure, instead of swallowing both and returning Ok(synthetic_txid). The caller can now distinguish "ARC accepted" from "ARC rejected" or "ARC unreachable."
2. R2 — WalletProvider::get_transaction parses cached transaction bytes (hex from the local tx_cache) and populates TransactionData.inputs / outputs, instead of always returning empty arrays. Cache-miss semantics are preserved unchanged (still Ok(TransactionData { raw: None, .. }) — locked by an explicit regression test).

Both fixes are local-minimal — no trait-signature changes, no new pub-surface besides what was strictly required to write the regression tests through public API.

Problem

R1 — silent broadcast failure

Upstream HEAD (packages/runar-rs/src/sdk/wallet.rs):

match ureq::post(&arc_endpoint)
    .set("Content-Type", "application/octet-stream")
    .send_bytes(&raw_bytes)
{
    Ok(resp) => {
        let body = resp.into_string().unwrap_or_default();
        if let Ok(json) = serde_json::from_str::<Value>(&body) {
            if let Some(arc_txid) = json.get("txid").and_then(|v| v.as_str()) {
                self.tx_cache.insert(arc_txid.to_string(), raw_hex);
                return Ok(arc_txid.to_string());
            }
        }
        self.tx_cache.insert(txid.clone(), raw_hex);
        Ok(txid)                                             // ← !
    }
    Err(_) => {
        // ARC unreachable — cache locally and return computed txid
        self.tx_cache.insert(txid.clone(), raw_hex);
        Ok(txid)                                             // ← !
    }
}

Both branches return Ok(...). On an HTTP non-2xx response (e.g. ARC HTTP 461 — "Script evaluated without error but finished with a false/empty top stack element" — the canonical NULLFAIL / preimage-mismatch rejection), the synthetic locally-computed txid is returned and the caller treats the broadcast as successful. On a network failure (connection refused, DNS error, TLS handshake failure) the same path runs — the SDK pretends a never-attempted broadcast succeeded.

This is a textbook silent-failure bug. The caller has no signal that the transaction is not on chain.

R2 — empty inputs/outputs on get_transaction

Upstream HEAD (packages/runar-rs/src/sdk/wallet.rs:323-345):

fn get_transaction(&self, txid: &str) -> Result<TransactionData, String> {
    if let Some(raw_hex) = self.tx_cache.get(txid) {
        return Ok(TransactionData {
            txid: txid.to_string(),
            raw: Some(raw_hex.clone()),
            inputs: vec![],                                  // ← always empty
            outputs: vec![],                                 // ← always empty
        });
    }
    // ... cache-miss fallback returns the same empty shape ...
}

On both cache hit and cache miss, inputs and outputs are returned as empty Vec. The SDK's higher-level RunarContract::from_txid(txid) then calls provider.get_transaction(txid) and indexes tx.outputs[output_index] to find the contract UTXO — indexing into an empty Vec panics, or (when wrapped in a .get(...) check) returns "output index 0 out of range (tx has 0 outputs)."

How this surfaced

R1: surfaced during a mainnet broadcast where the SDK reported Ok(txid), but a subsequent post-broadcast audit against chain state showed the transaction was never accepted by ARC — ARC had returned an HTTP 461 that the SDK silently swallowed. Caller-side reconciliation eventually caught the divergence, but only after multiple downstream actions had been keyed off the (non-existent) "broadcast" txid.

R2: surfaced when reloading a deployed stateful contract by its on-chain txid — RunarContract::from_txid(deployed_txid) could not retrieve the contract output, blocking every method call on every reloaded stateful contract. The SDK has parse_raw_tx in the same file; populating from it is straightforward.

Fix

R1 — surface broadcast failures:

let response = ureq::post(&arc_endpoint)
    .set("Content-Type", "application/octet-stream")
    .send_bytes(&raw_bytes);

let resp = match response {
    Ok(r) => r,
    Err(ureq::Error::Status(code, r)) => {
        let body = r.into_string().unwrap_or_default();
        return Err(format!(
            "WalletProvider broadcast: ARC HTTP {} from {}: {}",
            code, arc_endpoint, body
        ));
    }
    Err(e) => {
        return Err(format!(
            "WalletProvider broadcast: ARC network error against {}: {}",
            arc_endpoint, e
        ));
    }
};

let status_code = resp.status();
let body = resp.into_string().unwrap_or_default();
if !(200..300).contains(&status_code) {
    return Err(format!(
        "WalletProvider broadcast: ARC HTTP {} from {}: {}",
        status_code, arc_endpoint, body
    ));
}
// ... parse 2xx JSON body, extract arc-reported txid, cache, Ok(...) ...

R2 — populate inputs/outputs from cached bytes:

fn get_transaction(&self, txid: &str) -> Result<TransactionData, String> {
    if let Some(raw_hex) = self.tx_cache.get(txid) {
        let tx = BsvTransaction::from_hex(&raw_hex)
            .map_err(|e| format!("get_transaction parse: {}", e))?;
        return Ok(TransactionData {
            txid: txid.to_string(),
            raw: Some(raw_hex.clone()),
            inputs: tx.inputs.iter().map(|i| TxInput { /* mapped fields */ }).collect(),
            outputs: tx.outputs.iter().map(|o| TxOutput { /* mapped fields */ }).collect(),
        });
    }
    // Cache miss — preserve upstream's Ok(TransactionData { raw: None, .. }) shape
    Ok(TransactionData { txid: txid.to_string(), raw: None, inputs: vec![], outputs: vec![] })
}

The cache-miss branch is preserved verbatim from upstream — same Ok(TransactionData { raw: None, .. }) shape. A regression test (r2_get_transaction_cache_miss_preserves_upstream_fallback) locks this in so future refactors can't silently change cache-miss semantics.

Cross-language parity

TS runar-sdk/src/providers/wallet-provider.ts:196-217 parses tx_cache hex via Transaction.fromHex() and maps inputs/outputs onto the result — exactly the shape this PR ports into Rust. The TS broadcaster path already returns the equivalent error shape via thrown exceptions on non-2xx HTTP.

The strict canonical ecosystem shape for broadcast errors across @bsv/sdk (Broadcaster.ts:24-34) and b1narydt/bsv-rust-sdk (src/transaction/broadcaster.rs:33-56) is a structured BroadcastFailure { status, code, description, txid?, more? }. The runar-rs WalletProvider::broadcast trait signature today is Result<String, String> — adopting the structured shape would require a trait-signature change. This PR keeps the trait signature stable and surfaces the HTTP status + ARC body in the Err string. See ISSUE-02 — WalletProvider::with_broadcaster injection for the architectural follow-up that would let WalletProvider delegate to the canonical Broadcaster trait directly (and thus inherit the structured failure shape upstream).

Verification

Reproducible against upstream HEAD on a fresh clone in under a minute:

git clone --depth 1 https://github.com/icellan/runar /tmp/runar
cd /tmp/runar
git apply <runar-r1-r2-broadcast-and-get-transaction-clean.patch>
cd packages/runar-rs
cargo test --test wallet_provider_r1_r2_regression

Passes with patch:

running 4 tests
test r2_get_transaction_cache_miss_preserves_upstream_fallback ... ok
test r2_get_transaction_populates_inputs_and_outputs_from_cache ... ok
test r1_broadcast_returns_err_on_network_failure ... ok
test r1_broadcast_returns_err_on_http_461 ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Fails without patch (clean git stash of wallet.rs):

failures:

---- r2_get_transaction_populates_inputs_and_outputs_from_cache stdout ----
assertion `left == right` failed: R2 regression: get_transaction must populate inputs from raw bytes (not empty)
  left: 0
 right: 1

---- r1_broadcast_returns_err_on_network_failure stdout ----
R1 regression: broadcast() must return Err on connection refused, got Ok(Ok("be4a2327a866a86bb6c396cc8c232638b9bc2ac62b6cd3330237892f7098ca17"))

---- r1_broadcast_returns_err_on_http_461 stdout ----
R1 regression: broadcast() must return Err on ARC HTTP 461, got Ok(Ok("be4a2327a866a86bb6c396cc8c232638b9bc2ac62b6cd3330237892f7098ca17"))

test result: FAILED. 1 passed; 3 failed

The one passing pre-patch test (r2_get_transaction_cache_miss_preserves_upstream_fallback) intentionally locks upstream's unchanged behavior on cache miss — its post-patch pass proves the patch did NOT regress that fallback path.

Backwards compatibility

• Trait signature unchanged: WalletProvider::broadcast still returns Result<String, String>, get_transaction still returns Result<TransactionData, String>.
• Callers that previously branched on Ok(_) continue to compile. Callers that ignored the Err arm (relying on the silent-Ok behaviour to mask failures) will now see real errors propagate — which is the intended behaviour change.
• get_transaction cache-miss path is unchanged; a regression test (r2_get_transaction_cache_miss_preserves_upstream_fallback) locks the upstream Ok(TransactionData { raw: None, .. }) shape so future refactors can't silently regress it.
• No public-API additions beyond the imports the patch needs (TxInput, TxOutput were already exported from super::types).

Related

• ISSUE-02 — WalletProvider::with_broadcaster injection: the architectural follow-up that would adopt bsv-rust-sdk::Broadcaster upstream and inherit the canonical structured BroadcastFailure shape. R1 here is the local-minimal fix; ISSUE-02 is the long-term direction.
• ISSUE-03 — CallOptions::verify_inputs_on_chain: defensive opt-in pre-broadcast probe — addresses a related cross-broadcaster failure class that R1 helps surface (now that errors are visible, the probe can fail fast before submitting an orphan-bound child).