Wire post-deploy smoke and broadcast recovery E2E (#368) #877
No reviewers
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic!877
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/368-broadcast-recovery-smoke-qa"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
GitLab #368 — wires H12 (post-sign broadcast recovery E2E) and M12 (post-deploy smoke in QA bring-up).
RpcClient.broadcastTxintercept; on broadcast timeout/network failure, enterrecoveringphase, poll through msgdeadlinewithout re-broadcasting, show Broadcast status unknown… retail copy.terra-broadcast-recovery.spec.tsine2e-tx(Simulated Wallet; hungbroadcast_tx_sync→ recovery UX → success).make start-qarunssmoke-pool-swap.shafterqa-verify-deployusingscripts/lib/smoke-deploy-env.sh(pair from.qa-deploy-stamp, offer token from pool query).QA_SKIP_SMOKE=1skips.make smoke-pool-swapwrapper added.docs/testing.md,scripts/qa/README.md,AGENTS_FRONTEND_TX_BROADCAST_TIMEOUT.md,AGENTS_E2E_STRICT_CHAIN.md,AGENTS_TESTING_P2_EPIC.md.Acceptance checklist
CI=1 bash scripts/with-node.sh --cwd frontend-dapp -- ./node_modules/.bin/playwright test --project=e2e-tx terra-broadcast-recovery.spec.ts(aftermake setup-cloud-localterra+bash scripts/e2e-start-indexer.sh)make start-qaruns smoke after deploy verificationscripts/qa/start-qa.shinvokes smoke afterverify-deploy.sh(see[timing] smokephase)start-qanot re-run end-to-end on this VM after deploy)QA_SKIP_SMOKE=1 make start-qastill worksgrep QA_SKIP_SMOKE scripts/qa/start-qa.sh→ early skip logmake smoke-pool-swapon LocalTerra deploy stampskills/AGENTS_FRONTEND_TX_BROADCAST_TIMEOUT.md§ Post-sign broadcast recoveryVerification checklist (third parties)
make test-frontend— includesterraBroadcastRecovery.test.ts(935 tests passed on agent VM).make setup-cloud-localterra && bash scripts/e2e-start-indexer.shCI=1 make test-e2e-txor single spec:CI=1 bash scripts/with-node.sh --cwd frontend-dapp -- ./node_modules/.bin/playwright test --project=e2e-tx terra-broadcast-recovery.spec.tsmake smoke-pool-swap— expectOK: smoke-pool-swap read-only checks passed.make start-qalogs smoke success;QA_SKIP_SMOKE=1 make start-qaskips smoke line.Test plan notes
make test-frontendPASSterra-broadcast-recovery.spec.tsPASS (29.7s, LocalTerra + indexer)make smoke-pool-swapPASSchanged the description
changed the description
Stale Security Review comment
Stale Security Review comment
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit
ab877df57f. Configure here.Recovery expiry message not preserved
Low Severity
When
pollTerraTxRecoverythrowsTERRA_TX_RECOVERY_EXPIRED_MESSAGE, the outer catch runshandleBroadcastError, which does not pass that message through like broadcast or poll timeouts. Users get a genericTransaction failed:wrapper instead of the dedicated retry guidance copy.Additional Locations (1)
frontend-dapp/src/services/terraclassic/terraTxRecoveryPoll.ts#L38-L39Reviewed by Cursor Bugbot for commit
ab877df57f. Configure here.Recovery skipped after hash captured
Medium Severity
isPostSignBroadcastFailureonly treats broadcast timeout and a few network strings as recoverable onceinstallSignedTxHashCapturehas a hash. Other post-signErrorvalues skip therecoveringpoll path, so the mutation fails, UI re-enables swap, and the user may submit again while the first signed tx can still land on-chain.Additional Locations (1)
frontend-dapp/src/services/terraclassic/terraBroadcast.ts#L144-L152Reviewed by Cursor Bugbot for commit
ab877df57f. Configure here.changed this line in version 2 of the diff
added 1 commit
517f66da- fix: recover on any post-sign broadcast failure (#368)Compare with previous version
changed the description
resolved all threads
Severity: Medium
Issue: Post-sign recovery is gated on a narrow
isPostSignBroadcastFailureallowlist. AfterinstallSignedTxHashCapturerecords a hash, many real RPC failures (HTTP 5xx, ABCI/mempool reject strings, JSON parse errors, etc.) do not match the timeout/network regex and skip therecoveringpoll path.Impact: The mutation rejects,
useTerraBroadcastMutationresets phase inonSettled, and the Swap CTA re-enables while the already-signed tx can still be accepted by the network — duplicate swap / unintended second execution (user fund loss). A hostile or flaky RPC can return a non-matching error after a successful submit to surface this window.Evidence: Hash capture runs before
original(endpoint, txRaw)returns (terraWalletSignTxRaw.ts), but recovery only runs when the caught error is the exact broadcast-timeout string or matches/failed to fetch|networkerror|network error/i(terraBroadcast.tsL39–45, L145–152). All other post-sign errors rethrow immediately.Severity: Medium
Issue:
pollTerraTxRecoveryaborts the recovery window on the firstpollTxerror that is not a "tx not found" string, including transient network/LCD failures.Impact: While the UI is in
recovering(CTA disabled), a single poll network error ends recovery, the mutation fails, and the user can submit again even though the signed broadcast may already be in the mempool or confirmed — duplicate execution risk that this MR is meant to eliminate.Evidence: Only
not found/tx not founderrors are retried untildeadlineUnix; any otherErroris rethrown immediately (L31–33), propagating tobroadcastTerraExecuteContractsand resetting broadcast UI state viaonSettled.Security review — MR !877
Commit reviewed:
ab877df57f5b9cc0ab46171e2392cbf8be3e9840Scope: Post-sign Terra broadcast recovery (
terraBroadcast.ts,terraWalletSignTxRaw.ts,terraTxRecoveryPoll.ts,terraMsgDeadline.ts), recovery UX (TerraBroadcastPendingLink,useTerraBroadcastMutation,terraBroadcastUi), Playwrightterra-broadcast-recovery.spec.ts, QA smoke wiring (smoke-deploy-env.sh,start-qa.sh,make smoke-pool-swap), and related docs/skills.Outcome:
FINDINGS: 2mediumBoth findings concern incomplete post-sign recovery allowing a second swap submission while a signed transaction may still execute (duplicate-execution / user fund loss). No high/critical issues, injection, authz bypass, secret leakage, or SSRF/XSS paths identified in this diff. QA smoke additions are read-only LCD queries sourced from operator-controlled deploy artifacts (same trust model as existing
verify-deploy.shstamp sourcing).Inline threads:
frontend-dapp/src/services/terraclassic/terraBroadcast.ts— narrowisPostSignBroadcastFailureallowlist skips recovery for many post-sign RPC errors after hash capture.frontend-dapp/src/services/terraclassic/terraTxRecoveryPoll.ts— recovery poll aborts on transient non–not-found errors instead of holding until the msg deadline.Label
block:securityapplied.Stale Security Review comment
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
517f66daa2. Configure here.Over-broad post-sign recovery trigger
Medium Severity
isPostSignBroadcastFailurenow treats any broadcast error as recoverable wheneversignedTxHashwas captured, but the hash is set as soon asRpcClient.broadcastTxruns, including immediate RPC rejections. Definite failures (e.g. sequence mismatch) enterrecoveringandpollTerraTxRecoveryuntil the msg deadline instead of surfacing the original error promptly.Reviewed by Cursor Bugbot for commit
517f66daa2. Configure here.added 1 commit
b6265934- fix: narrow post-sign broadcast recovery to timeout/network errors (#368)Compare with previous version
changed the description
mentioned in issue #368
added 1 commit
c02f1030- fix: harden post-sign broadcast recovery against duplicate swap (#368)Compare with previous version
Security fixes (commit
c02f103)isPostSignBroadcastFailureallowlist skipped recovery for many post-sign RPC errors after hash captureisDefiniteBroadcastRejection: sequence mismatch, signature/chain-id/decode/fee rejections).pollTerraTxRecoveryaborted on first non–not-found poll errorTerraOnChainBroadcastFailure(included tx with non-zero code) exits early.517f66d) sent definite rejections into recovery pollrecovering.handleBroadcastErrorhandleBroadcastError; added unit regressions for expiry copy end-to-end.Tests:
terraBroadcastRecovery.test.ts(15 passed).Removing
block:security— all low+ findings addressed.changed the description
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
c02f103081. Configure here.Cache duplicate skips recovery polling
Medium Severity
isDefiniteBroadcastRejectiontreats RPC errors matchingtx already exists in cacheas non-recoverable CheckTx failures. That response usually means the signed transaction is already in the mempool. With a captured hash, recovery is skipped, the mutation fails and the Swap CTA re-enables while the original tx may still confirm—undermining post-sign recovery and duplicate-execution protection.Reviewed by Cursor Bugbot for commit
c02f103081. Configure here.Severity: Medium
Issue:
isDefiniteBroadcastRejectiontreatstx already exists in cacheas a non-recoverable CheckTx failure. That RPC response indicates the signed transaction is already in the mempool cache (duplicate submit), not that the tx was rejected.Impact: After
installSignedTxHashCapturerecords the hash, this error skips therecoveringpoll path. The mutation rejects,useTerraBroadcastMutationresets inonSettled, and the Swap/Limit CTA re-enables while the original signed tx may still confirm — duplicate execution / user fund loss. A flaky RPC or client rebroadcast can surface this window.Evidence: Hash is captured before
original(endpoint, txRaw)returns (terraWalletSignTxRaw.tsL36–38).isPostSignBroadcastFailurereturns false whenisDefiniteBroadcastRejectionmatches (L58–61), so recovery is skipped. This pattern is the inverse of the #368 fix intent: the tx is already submitted and should be polled, not treated as a definite rejection.changed this line in version 5 of the diff
changed this line in version 5 of the diff
added 1 commit
03904029- fix: poll recovery when mempool reports tx already in cache (#368)Compare with previous version
Security review — MR !877
Commit reviewed:
c02f103081663affe9adfefa9e4f1c153a164f3fScope: Post-sign Terra broadcast recovery (
terraBroadcast.ts,terraWalletSignTxRaw.ts,terraTxRecoveryPoll.ts,terraMsgDeadline.ts), recovery UX (TerraBroadcastPendingLink,useTerraBroadcastMutation,terraBroadcastUi), Playwrightterra-broadcast-recovery.spec.ts, QA smoke wiring (smoke-deploy-env.sh,start-qa.sh,make smoke-pool-swap), and related docs/skills.Outcome:
FINDINGS: 1mediumThe two prior medium findings (narrow post-sign recovery allowlist; poll abort on transient errors) are resolved in
c02f103— broad recovery after hash capture withisDefiniteBroadcastRejectiongate, and poll retries until msg deadline. One new medium issue remains in the rejection classifier.Inline threads:
frontend-dapp/src/services/terraclassic/terraBroadcast.tsL50 —tx already exists in cachemisclassified as definite rejection; skips recovery while tx may already be in mempool.No high/critical issues, injection, authz bypass, secret leakage, or SSRF/XSS paths identified in this diff. QA smoke additions are read-only LCD queries sourced from operator-controlled deploy artifacts (same trust model as existing
verify-deploy.shstamp sourcing).changed the description
added 1 commit
1f36acb5- test: regression for mempool cache duplicate recovery (#368)Compare with previous version
Security fixes (commits
0390402,1f36acb)isPostSignBroadcastFailureallowlist skipped recovery for many post-sign RPC errors after hash captureisDefiniteBroadcastRejection: sequence mismatch, signature/chain-id/decode/fee rejections).pollTerraTxRecoveryaborted on first non–not-found poll errorTerraOnChainBroadcastFailure(included tx with non-zero code) exits early.recovering.handleBroadcastErrorhandleBroadcastError; unit regressions for expiry copy end-to-end.tx already exists in cachemisclassified as definite rejection; skipped recovery while tx may already be in mempoolisDefiniteBroadcastRejection(0390402); mempool duplicate now entersrecoveringpoll. Regression test added (1f36acb).Tests:
terraBroadcastRecovery.test.ts(16 passed).All low+ findings addressed. Removing
block:security.changed the description
Security review — MR !877
Commit reviewed:
1f36acb5ad484330d17bd945adb58ea761975eb0Scope: Post-sign Terra broadcast recovery (
terraBroadcast.ts,terraWalletSignTxRaw.ts,terraTxRecoveryPoll.ts,terraMsgDeadline.ts), recovery UX (TerraBroadcastPendingLink,useTerraBroadcastMutation,terraBroadcastUi), Playwrightterra-broadcast-recovery.spec.ts, QA smoke wiring (smoke-deploy-env.sh,start-qa.sh,make smoke-pool-swap), and related docs/skills.Outcome:
FINDINGS: 0medium+Security review: no medium+ findings on this diff.
Re-checked prior threads against the current head:
1f36acbisPostSignBroadcastFailureallowlist skipped recovery after hash capturesignedTxHashis set, gated byisDefiniteBroadcastRejection(sequence/signature/chain/decode/fee rejections only).pollTerraTxRecoveryaborted on transient poll errorsTerraOnChainBroadcastFailure(included tx, non-zero code) exits early.isDefiniteBroadcastRejectionfast-fail; unit test for sequence mismatch.tx already exists in cachemisclassified as definite rejection0390402, regression in1f36acb) — mempool duplicate now entersrecoveringand polls.Additional checks (no issues):
swapMutation.isPendingkeeps Swap CTA disabled throughrecovering.txHashfor explorer links is derived from signedTxRaw(hex); no XSS/injection sink.verify-deploy.sh).Inline threads: none (no current medium+ findings).
changed this line in version 7 of the diff
changed this file in version 7 of the diff
changed this line in version 7 of the diff
changed this line in version 7 of the diff
added 55 commits
main337f6bec- Merge origin/main into feat/368-broadcast-recovery-smoke-qaCompare with previous version
Resolved merge conflicts with
main(337f6be).Broadcast recovery: Took main's split sign/broadcast path (#359) —
signTerraTxRaw,pollTxUntilRecoveryDeadline,terraBroadcastPendingStatusMessage— replacing the branch's older RpcClient intercept approach. Mempool-cache recovery behavior is already covered on main.#368 retained: smoke QA wiring (
make smoke-pool-swap,start-qa), E2E spec, and docs/skills updates.Sanity: 13 unit tests pass (
terraBroadcastRecovery,terraBroadcastUi,TerraBroadcastPendingLink).changed the description
Security review — MR !877
Commit reviewed:
337f6becc2afb9c8a1e94a2cdceb8b1bed1215f8Scope: Post-merge #368 wiring — QA smoke (
scripts/lib/smoke-deploy-env.sh,make smoke-pool-swap,scripts/qa/start-qa.sh), Playwrightterra-broadcast-recovery.spec.ts+playwright.config.tstimeout/glob, orphanedservices/terraclassic/terraMsgDeadline.ts, test mock tweak, docs/skills. Broadcast-recovery implementation (terraBroadcast.ts,terraWalletSignTxRaw.ts,terraTxRecoveryPoll.ts,utils/terraMsgDeadline.ts) is onmain(#359); exercised by the new E2E but not modified in this diff.Outcome:
FINDINGS: 0medium+Security review: no medium+ findings on this diff.
Re-check of prior threads (against current head):
337f6bemain(shouldRecoverPostSignBroadcast+isDefiniteBroadcastRejectiongate)main(pollTxUntilRecoveryDeadlineretries until deadline)tx already exists in cachemisclassifiedmain(pattern removed from rejection classifier)Additional checks on this diff (no issues):
PAIR_ADDR/OFFER_TOKENresolved from deploy stamp or on-chainpoolresponse (same trust model as existingverify-deploy.sh). No secrets echoed, no user-facing attack surface.VITE_TERRA_TX_BROADCAST_TIMEOUT_MS=3000: PlaywrightwebServerenv only; production builds unaffected.services/terraclassic/terraMsgDeadline.ts: Dead duplicate (production usesutils/terraMsgDeadline.tsviaresolveTerraTxRecoveryDeadlineUnix); docs reference wrong path — maintenance drift, not an exploitable sink.Inline threads: none (no current medium+ findings).
mentioned in commit
8c641d165cmentioned in issue #337