You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
TxStatus.Deleted currently represents two semantically distinct outcomes that callers cannot tell apart without inspecting the free-text message column:
Ledger-rejected / invalid — set by the finality listener when the token request hash check fails (token/services/ttx/finality/listener.go:138, token/services/ttx/finality/recovery.go:122) or when the network returns Invalid (listener.go:148, recovery.go:133). The transaction was actively rejected; there is nothing to retry.
Never reached the ledger (orphan) — set by the recovery loop when GetTransactionStatus keeps returning NotFound past NotFoundGracePeriod (token/services/storage/recovery/manager.go:331, introduced in fix(recovery): unblock queue head by marking NotFound orphans Deleted after grace period #1708). The transaction's audit log was persisted but the broadcast / commit never landed; in principle the request could be replayed.
These two cases require very different operator responses, but in storage they are indistinguishable beyond the message string.
Why this matters
Without a dedicated state for the orphan case:
Operational dashboards / queries cannot count or surface "broadcast failures" separately from "rejected transactions".
Any future second-chance workflow (manual replay, automatic re-broadcast) has no clean column to filter on — it would have to parse the message column to decide which Deleted rows are candidates.
The downstream consumers that act on Deleted (token-lock cleanup, in-memory locker, finality status checks) currently apply the same termination semantics to both cases, even though an orphan with retry potential should keep its input locks held until the retry window expires.
I would also probe your opinion on introducing a new state for this transactions that are kind of orphaned. This would allows us to find them more easily and give them a second chance.
@adecaro to make sure I'm reading you right — is the proposal to change the SetStatus(ctx, txID, storage.Deleted, ...) call I added in #1708 (recovery/manager.go:331, on the NotFoundGracePeriod path) so it sets a new dedicated Orphan state instead? That way Deleted stays reserved for explicit operator/user deletion, and "tried to commit, never landed on chain within the grace window" gets its own unambiguous signal.
Yes, this is what I meant. We can introduce this new state in a new PR. That's also fine.
This issue tracks that follow-up.
Proposed change
Add Orphan as a dedicated TxStatus value alongside Deleted, and route the recovery loop's NotFound-past-grace path to it. Keep Deleted reserved for ledger-rejected / hash-mismatch cases.
The retry / rebroadcast workflow itself (promoting Orphan back to Pending or to a dedicated Retried state). That requires its own design discussion and is meaningful only once the Orphan value exists.
Behaviour changes to the downstream Deleted-checking call sites (token-lock cleanup, in-memory locker, db/common/checks.go, ttx/finality.go). feat(recovery): promote NotFound orphans to dedicated Orphan TxStatus #1722 keeps those identical so the state introduction is strictly additive; consumer-side semantics for Orphan will be tuned in follow-up PRs as the second-chance workflow takes shape.
The residual race in the recovery SetStatus write described in recovery/manager.go:318-324 — replacing the unconditional update with an atomic status=Pending → Orphan CAS at the SQL layer is a separate hardening step.
Problem
TxStatus.Deletedcurrently represents two semantically distinct outcomes that callers cannot tell apart without inspecting the free-text message column:token/services/ttx/finality/listener.go:138,token/services/ttx/finality/recovery.go:122) or when the network returnsInvalid(listener.go:148,recovery.go:133). The transaction was actively rejected; there is nothing to retry.GetTransactionStatuskeeps returningNotFoundpastNotFoundGracePeriod(token/services/storage/recovery/manager.go:331, introduced in fix(recovery): unblock queue head by marking NotFound orphans Deleted after grace period #1708). The transaction's audit log was persisted but the broadcast / commit never landed; in principle the request could be replayed.These two cases require very different operator responses, but in storage they are indistinguishable beyond the message string.
Why this matters
Without a dedicated state for the orphan case:
Deletedrows are candidates.Deleted(token-lock cleanup, in-memory locker, finality status checks) currently apply the same termination semantics to both cases, even though an orphan with retry potential should keep its input locks held until the retry window expires.Context from review thread
This was raised by @adecaro during review of #1715:
I confirmed the intended scope:
And @adecaro agreed to land it as a follow-up PR:
This issue tracks that follow-up.
Proposed change
Add
Orphanas a dedicatedTxStatusvalue alongsideDeleted, and route the recovery loop's NotFound-past-grace path to it. KeepDeletedreserved for ledger-rejected / hash-mismatch cases.Implementation in #1722.
Out of scope
Orphanback toPendingor to a dedicatedRetriedstate). That requires its own design discussion and is meaningful only once theOrphanvalue exists.Deleted-checking call sites (token-lock cleanup, in-memory locker,db/common/checks.go,ttx/finality.go). feat(recovery): promote NotFound orphans to dedicated Orphan TxStatus #1722 keeps those identical so the state introduction is strictly additive; consumer-side semantics forOrphanwill be tuned in follow-up PRs as the second-chance workflow takes shape.SetStatuswrite described inrecovery/manager.go:318-324— replacing the unconditional update with an atomicstatus=Pending → OrphanCAS at the SQL layer is a separate hardening step.