code/cl8y-dex-terraclassic

Fork 0

Fix post-sign broadcast timeout double-execution risk (#359) #867

Merged

PlasticDigits merged 2 commits from fix/359-post-sign-broadcast-recovery into main

2026-06-12 03:34:43 +00:00

PlasticDigits commented

2026-06-11 15:33:52 +00:00

(Migrated from gitlab.com)

Summary

Fixes #359: after the wallet signs, a hung RPC broadcast no longer shows a false "Could not broadcast… try again" message that invited double-execution inside the swap deadline window.

Split sign / broadcast for extension + dev mnemonic wallets (terraWalletSignTxRaw.ts): compute tx hash from signed bytes before RPC submit.
Post-sign recovery: on broadcast or poll timeout with a known hash, enter phase recovering, show honest "Broadcast status unknown — the transaction may still confirm" copy, poll LCD through the msg deadline (or 300s default), and only then offer retry.
Pre-sign failures (atomic WC post path, user reject before sign) keep existing TERRA_TX_BROADCAST_TIMEOUT_MESSAGE.
Docs/skills cross-linked: docs/frontend.md, AGENTS_FRONTEND_TX_BROADCAST_TIMEOUT.md, AGENTS_FRONTEND_USER_ERRORS.md.

Acceptance checklist

Criterion	Verification	Result
Post-sign hung RPC does not claim "could not broadcast" while tx may still land	`npm run test:unit -- src/services/terraclassic/__tests__/terraBroadcastRecovery.test.ts` (broadcast timeout → `recovering` → success)	PASS
Tx hash computed from signed bytes before broadcast	`npm run test:unit -- src/utils/__tests__/terraTxHash.test.ts` + split-path sign in `terraWalletSignTxRaw.ts`	PASS
Poll LCD through deadline before re-enabling submit	Recovery test + `pollTxUntilRecoveryDeadline`; submit stays disabled via `isPending` until resolve	PASS
Only offer retry after definitive not-found	`terraBroadcastRecovery.test.ts` expects `TERRA_TX_POST_SIGN_NOT_FOUND_MESSAGE`	PASS
Pre-sign connection failures keep safe retry copy	`transactions.test.ts` broadcast timeout on atomic mock wallet (no split)	PASS
UI surfaces unknown-status copy during recovery	`TerraBroadcastPendingLink.test.tsx` + `terraBroadcastUi.test.ts`	PASS
Full frontend unit regression	`npm run test:unit` (934 tests)	PASS
Manual hung-RPC swap on LocalTerra	Requires `make setup-cloud-localterra` + DevTools offline after wallet approve on RPC only	SKIP (not run in agent VM this session)

Third-party verification

cd frontend-dapp
npm run test:unit -- src/services/terraclassic/__tests__/terraBroadcastRecovery.test.ts \
  src/utils/__tests__/terraMsgDeadline.test.ts \
  src/components/ui/__tests__/TerraBroadcastPendingLink.test.tsx
npm run test:unit   # full suite

Manual (optional): LocalTerra + Simulated Wallet → /swap → approve swap → block LCD/RPC (not wallet) → expect Broadcast status unknown…, button disabled with Checking broadcast…, then success if chain unfreezes before deadline.

Closes #359 when merged (issue left open per implement workflow).

Note

High Risk
Touches the canonical on-chain submit path for all Terra executes; wrong recovery or retry timing could still double-spend swaps or leave users stuck, though regression tests cover main timeout/recovery cases.

Overview
Fixes #359 by changing how Terra Classic submits behave when the wallet has already signed but RPC broadcast or confirmation hangs.

Split sign / broadcast for extension and dev mnemonic wallets: sign offline, derive the tx hash from signed bytes, then submit via RpcClient.broadcastTx. Atomic WalletConnect post paths stay on the old wallet.broadcastTx flow with pre-sign timeout copy only.

When broadcast or pollTx times out after a hash exists, the flow enters phase recovering instead of treating it like a failed broadcast. The UI shows honest "Broadcast status unknown…" copy, keeps submit disabled (Checking broadcast…), and pollTxUntilRecoveryDeadline polls LCD until the swap msg deadline (or 300s default). Retry is offered only after TERRA_TX_POST_SIGN_NOT_FOUND_MESSAGE.

Supporting pieces: terraWalletSignTxRaw, terraMsgDeadline, terraTxHash, handleBroadcastError / isTerraTxTimeoutMessage for new pass-through messages, useTerraBroadcastMutation + TerraBroadcastPendingLink for recovery status. Docs and agent skills updated.

^{Reviewed by Cursor Bugbot for commit 839ab90a61. Bugbot is set up for automated code reviews on this repo. Configure here.}

## Summary Fixes [#359](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/359): after the wallet signs, a hung RPC broadcast no longer shows a false "Could not broadcast… try again" message that invited double-execution inside the swap deadline window. - **Split sign / broadcast** for extension + dev mnemonic wallets (`terraWalletSignTxRaw.ts`): compute tx hash from signed bytes before RPC submit. - **Post-sign recovery**: on broadcast or poll timeout with a known hash, enter phase `recovering`, show honest *"Broadcast status unknown — the transaction may still confirm"* copy, poll LCD through the msg `deadline` (or 300s default), and only then offer retry. - **Pre-sign failures** (atomic WC `post` path, user reject before sign) keep existing `TERRA_TX_BROADCAST_TIMEOUT_MESSAGE`. - Docs/skills cross-linked: `docs/frontend.md`, `AGENTS_FRONTEND_TX_BROADCAST_TIMEOUT.md`, `AGENTS_FRONTEND_USER_ERRORS.md`. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | Post-sign hung RPC does not claim "could not broadcast" while tx may still land | `npm run test:unit -- src/services/terraclassic/__tests__/terraBroadcastRecovery.test.ts` (broadcast timeout → `recovering` → success) | **PASS** | | Tx hash computed from signed bytes before broadcast | `npm run test:unit -- src/utils/__tests__/terraTxHash.test.ts` + split-path sign in `terraWalletSignTxRaw.ts` | **PASS** | | Poll LCD through deadline before re-enabling submit | Recovery test + `pollTxUntilRecoveryDeadline`; submit stays disabled via `isPending` until resolve | **PASS** | | Only offer retry after definitive not-found | `terraBroadcastRecovery.test.ts` expects `TERRA_TX_POST_SIGN_NOT_FOUND_MESSAGE` | **PASS** | | Pre-sign connection failures keep safe retry copy | `transactions.test.ts` broadcast timeout on atomic mock wallet (no split) | **PASS** | | UI surfaces unknown-status copy during recovery | `TerraBroadcastPendingLink.test.tsx` + `terraBroadcastUi.test.ts` | **PASS** | | Full frontend unit regression | `npm run test:unit` (934 tests) | **PASS** | | Manual hung-RPC swap on LocalTerra | Requires `make setup-cloud-localterra` + DevTools offline **after** wallet approve on RPC only | **SKIP** (not run in agent VM this session) | ## Third-party verification ```bash cd frontend-dapp npm run test:unit -- src/services/terraclassic/__tests__/terraBroadcastRecovery.test.ts \ src/utils/__tests__/terraMsgDeadline.test.ts \ src/components/ui/__tests__/TerraBroadcastPendingLink.test.tsx npm run test:unit # full suite ``` **Manual (optional):** LocalTerra + Simulated Wallet → `/swap` → approve swap → block LCD/RPC (not wallet) → expect *Broadcast status unknown…*, button disabled with *Checking broadcast…*, then success if chain unfreezes before deadline. Closes #359 when merged (issue left open per implement workflow).  --- > [!NOTE] > **High Risk** > Touches the canonical on-chain submit path for all Terra executes; wrong recovery or retry timing could still double-spend swaps or leave users stuck, though regression tests cover main timeout/recovery cases. > > **Overview** > Fixes **#359** by changing how Terra Classic submits behave when the wallet has already signed but RPC broadcast or confirmation hangs. > > **Split sign / broadcast** for extension and dev mnemonic wallets: sign offline, derive the tx hash from signed bytes, then submit via `RpcClient.broadcastTx`. Atomic WalletConnect `post` paths stay on the old `wallet.broadcastTx` flow with pre-sign timeout copy only. > > When broadcast or `pollTx` times out **after** a hash exists, the flow enters phase **`recovering`** instead of treating it like a failed broadcast. The UI shows honest *"Broadcast status unknown…"* copy, keeps submit disabled (*Checking broadcast…*), and **`pollTxUntilRecoveryDeadline`** polls LCD until the swap msg **`deadline`** (or **300s** default). Retry is offered only after **`TERRA_TX_POST_SIGN_NOT_FOUND_MESSAGE`**. > > Supporting pieces: **`terraWalletSignTxRaw`**, **`terraMsgDeadline`**, **`terraTxHash`**, **`handleBroadcastError`** / **`isTerraTxTimeoutMessage`** for new pass-through messages, **`useTerraBroadcastMutation`** + **`TerraBroadcastPendingLink`** for recovery status. Docs and agent skills updated. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 839ab90a612c162f5523cc1a40ff99f428cc8a53. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>

ghost1 commented

2026-06-11 15:34:00 +00:00

(Migrated from gitlab.com)

changed the description

ghost1 commented

2026-06-11 15:34:10 +00:00

(Migrated from gitlab.com)

changed the description

ghost1 commented

2026-06-11 15:35:47 +00:00

(Migrated from gitlab.com)

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

^{Reviewed by Cursor Bugbot for commit cd178131ad. Configure here.}

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.  <a href="https://cursor.com/open?link=eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9BTExfSU5fQ1VSU09SIiwiZGF0YSI6eyJyZWRpc0tleSI6ImJ1Z2JvdC1tdWx0aTpkNzA1MGNlMS0wZmFhLTQ4NWItYmJkOS01ZGNiODgwZGZjOGIiLCJlbmNyeXB0aW9uS2V5IjoiaTBqejVDTXMxRU1uMm9qNjQyVVJPZXZRTUo4a3R6MXEyTHEyQy16WmFyTSIsImJyYW5jaCI6ImZpeC8zNTktcG9zdC1zaWduLWJyb2FkY2FzdC1yZWNvdmVyeSIsInJlcG9Pd25lciI6IlBsYXN0aWNEaWdpdHMiLCJyZXBvTmFtZSI6ImNsOHktZGV4LXRlcnJhY2xhc3NpYyJ9fQ" target="_blank" rel="noopener noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/fix-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/fix-in-cursor-light.png"><img alt="Fix All in Cursor" width="115" height="28" src="https://cursor.com/assets/images/fix-in-cursor-dark.png"></picture></a>   <sup>Bugbot Autofix is [ON](https://www.cursor.com/dashboard/bugbot). A cloud agent has been kicked off to fix the reported issue. </sup>  <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cd178131adbb7309523287177500a068646e8730. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>

ghost1 commented

2026-06-11 15:35:48 +00:00

(Migrated from gitlab.com)

Recovery skips LCD at deadline

High Severity

When recovery polling reaches the swap deadline from the signed msg, pollTxUntilRecoveryDeadline throws TERRA_TX_POST_SIGN_NOT_FOUND_MESSAGE without calling getTx, so a transaction that already confirmed can still surface as not found and invite retry.

^{Reviewed by Cursor Bugbot for commit cd178131ad. Configure here.}

### Recovery skips LCD at deadline **High Severity**  When recovery polling reaches the swap `deadline` from the signed msg, `pollTxUntilRecoveryDeadline` throws `TERRA_TX_POST_SIGN_NOT_FOUND_MESSAGE` without calling `getTx`, so a transaction that already confirmed can still surface as not found and invite retry.    <div><a href="https://cursor.com/open?link=eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9JTl9DVVJTT1IiLCJkYXRhIjp7InJlZGlzS2V5IjoiYnVnYm90OmNhNTU0OTY2LTgyNWEtNDRmMi1iZTU1LWE5NzlkYThiYTZmNCIsImVuY3J5cHRpb25LZXkiOiJsc3ByalE2ZUoyaUQya1p6ZFpaWGtXZGpINldHVTJBN1VpSkRnbjNLX013IiwiYnJhbmNoIjoiZml4LzM1OS1wb3N0LXNpZ24tYnJvYWRjYXN0LXJlY292ZXJ5IiwicmVwb093bmVyIjoiUGxhc3RpY0RpZ2l0cyIsInJlcG9OYW1lIjoiY2w4eS1kZXgtdGVycmFjbGFzc2ljIn19" target="_blank" rel="noopener noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/fix-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/fix-in-cursor-light.png"><img alt="Fix in Cursor" width="115" height="28" src="https://cursor.com/assets/images/fix-in-cursor-dark.png"></picture></a> <a href="https://cursor.com/agents?link=eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9JTl9XRUIiLCJkYXRhIjp7InJlZGlzS2V5IjoiYnVnYm90OmNhNTU0OTY2LTgyNWEtNDRmMi1iZTU1LWE5NzlkYThiYTZmNCIsImVuY3J5cHRpb25LZXkiOiJsc3ByalE2ZUoyaUQya1p6ZFpaWGtXZGpINldHVTJBN1VpSkRnbjNLX013IiwiYnJhbmNoIjoiZml4LzM1OS1wb3N0LXNpZ24tYnJvYWRjYXN0LXJlY292ZXJ5IiwicmVwb093bmVyIjoiUGxhc3RpY0RpZ2l0cyIsInJlcG9OYW1lIjoiY2w4eS1kZXgtdGVycmFjbGFzc2ljIiwicHJOdW1iZXIiOjE0OSwiY29tbWl0U2hhIjoiY2QxNzgxMzFhZGJiNzMwOTUyMzI4NzE3NzUwMGEwNjg2NDZlODczMCIsInByb3ZpZGVyIjoiZ2l0bGFiIiwiaG9zdG5hbWUiOiJnaXRsYWIuY29tIn19" target="_blank" rel="noopener noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/fix-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/fix-in-web-light.png"><img alt="Fix in Web" width="99" height="28" src="https://cursor.com/assets/images/fix-in-web-dark.png"></picture></a></div> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cd178131adbb7309523287177500a068646e8730. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>

PlasticDigits commented

2026-06-11 15:36:17 +00:00

(Migrated from gitlab.com)

Security review

Commit reviewed: cd178131adbb7309523287177500a068646e8730
Scope: MR !867 — post-sign broadcast timeout recovery (#359): split sign/broadcast (terraWalletSignTxRaw.ts), LCD deadline polling (terraTxRecoveryPoll.ts, terraMsgDeadline.ts), recovery UI phases, timeout copy.

Outcome: FINDINGS: 0 medium+

Reviewed added/modified frontend broadcast paths for injection, authz bypass, secret leakage, unsafe deserialization, SSRF/XSS, and double-execution regressions. Prior MR security threads: none.

Summary

This change reduces double-execution risk by keeping submit disabled through an honest recovery phase after post-sign RPC/poll timeouts, deriving the tx hash from signed bytes before broadcast, and polling LCD until the on-chain msg deadline (or 300s default) before offering retry.

No medium+ issues identified on this diff:

Tx hash / explorer link: Hash is SHA-256 of protobuf TxRaw (fixed hex); React text/href sinks are not attacker-controlled.
LCD poll endpoint: wallet.rpc is the connected wallet’s configured endpoint (same trust model as existing pollTx / getTx usage); txHash is not user-supplied.
Deadline extraction: Parsed only from the same entries used to build the signed execute msgs; finite-number checks; malformed CW20 inner msg falls back to default recovery window — no injection path to sinks.
Split sign/broadcast: Reuses existing extension fee-undershoot guards and sign lock; bumpWalletCachedSequence is deferred on broadcast timeout until recovery confirms inclusion, avoiding premature retry with a stale sequence.
Atomic WalletConnect post exclusion: Station/LuncDash WC retain the pre-MR path (documented); not a new attack surface introduced by this diff.

Inline threads: none (no findings to anchor).

## Security review **Commit reviewed:** `cd178131adbb7309523287177500a068646e8730` **Scope:** MR !867 — post-sign broadcast timeout recovery (#359): split sign/broadcast (`terraWalletSignTxRaw.ts`), LCD deadline polling (`terraTxRecoveryPoll.ts`, `terraMsgDeadline.ts`), recovery UI phases, timeout copy. **Outcome:** `FINDINGS: 0` medium+ Reviewed added/modified frontend broadcast paths for injection, authz bypass, secret leakage, unsafe deserialization, SSRF/XSS, and double-execution regressions. Prior MR security threads: none. ### Summary This change **reduces** double-execution risk by keeping submit disabled through an honest recovery phase after post-sign RPC/poll timeouts, deriving the tx hash from signed bytes before broadcast, and polling LCD until the on-chain msg `deadline` (or 300s default) before offering retry. **No medium+ issues identified on this diff:** - **Tx hash / explorer link:** Hash is SHA-256 of protobuf `TxRaw` (fixed hex); React text/`href` sinks are not attacker-controlled. - **LCD poll endpoint:** `wallet.rpc` is the connected wallet’s configured endpoint (same trust model as existing `pollTx` / `getTx` usage); `txHash` is not user-supplied. - **Deadline extraction:** Parsed only from the same `entries` used to build the signed execute msgs; finite-number checks; malformed CW20 inner `msg` falls back to default recovery window — no injection path to sinks. - **Split sign/broadcast:** Reuses existing extension fee-undershoot guards and sign lock; `bumpWalletCachedSequence` is deferred on broadcast timeout until recovery confirms inclusion, avoiding premature retry with a stale sequence. - **Atomic WalletConnect `post` exclusion:** Station/LuncDash WC retain the pre-MR path (documented); not a new attack surface introduced by this diff. **Inline threads:** none (no findings to anchor).

ghost1 commented

2026-06-11 15:38:17 +00:00

(Migrated from gitlab.com)

changed this line in version 2 of the diff

changed this line in [version 2 of the diff](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/149/diffs?diff_id=1847285561&start_sha=cd178131adbb7309523287177500a068646e8730#8ec5b7817a66b52114ab84d287254f35585d818d_22_21)

ghost1 commented

2026-06-11 15:38:17 +00:00

(Migrated from gitlab.com)

added 1 commit

839ab90a - fix: query LCD before deadline expiry in recovery poll (#359)

Compare with previous version

added 1 commit <ul><li>839ab90a - fix: query LCD before deadline expiry in recovery poll (#359)</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/149/diffs?diff_id=1847285561&start_sha=cd178131adbb7309523287177500a068646e8730)

ghost1 commented

2026-06-11 15:38:31 +00:00

(Migrated from gitlab.com)

changed the description

ghost1 commented

2026-06-11 15:38:42 +00:00

(Migrated from gitlab.com)

resolved all threads