ExecuteTransaction verification robustness follow-ups (head-of-line blocking, timeout misclassification, Tier-3 re-scan)

[jorgecuesta]

Summary

Tracking issue for verification-robustness follow-ups surfaced during the PR #278 review. None are regressions introduced by that PR's core fix; they're pre-existing/edge weaknesses in the ExecuteTransaction verification flow worth a dedicated pass.

Items

• Head-of-line blocking in ExecutePendingTransactions. It awaits each executeChild("ExecuteTransaction") sequentially in a for loop. With the new per-block verify retry policy (up to TX_EXPIRATION_BLOCKS × 60s ≈ 30 min on the not-found path), a single slow/stuck transaction now stalls every later pending transaction in that run. Consider bounded concurrency (e.g. Promise.all over batches, like SupplierStatus).

• verifyTransaction startToCloseTimeout (30s) vs. Tier-3 scan duration. One attempt can run Tier-1 + Tier-2 (REST) + Tier-3 (up to 30 sequential comet.block round-trips). A slow attempt is killed with a START_TO_CLOSE TimeoutFailure, whose cause is not the TX_NOT_FOUND ApplicationFailure that isTxNotFoundFailure() checks for. On the final attempt this flips verifyErroredUnexpectedly = true, so a merely-not-yet-landed tx is logged as verification errored (not a clean not-found) ... marked failure for triage — a misleading triage signal. The timeout also consumes a maximumAttempts slot, shrinking the real verify budget below 30 blocks.

• Tier-3 re-scans from scratch on every retry. For a never-landing tx, ~30 retries × up to 30 sequential comet.block(h) fetches ≈ ~900 block RPC round-trips against one endpoint over ~30 min, plus a fresh client connection per attempt, mostly re-reading already-scanned blocks. Parallelize the block fetches (Promise.all) and/or only scan newly-produced blocks since the previous attempt.

Notes

The block-count↔retry-count coupling (maximumAttempts = TX_EXPIRATION_BLOCKS with a 60s interval, assuming ~60s block time) is a related fragility: if block time drifts, the verify window no longer matches the on-chain mempool-expiration window. Worth considering a height-based loop instead of a time-based proxy.

[miguel502]

Summary

Tracking issue for verification-robustness follow-ups surfaced during the PR #278 review. None are regressions introduced by that PR's core fix; they're pre-existing/edge weaknesses in the ExecuteTransaction verification flow worth a dedicated pass.

Items

* [ ] 
  [ ]  **Head-of-line blocking in `ExecutePendingTransactions`.** It `await`s each `executeChild("ExecuteTransaction")` sequentially in a `for` loop. With the new per-block verify retry policy (up to `TX_EXPIRATION_BLOCKS` × 60s ≈ 30 min on the not-found path), a single slow/stuck transaction now stalls every later pending transaction in that run. Consider bounded concurrency (e.g. `Promise.all` over batches, like `SupplierStatus`).
  [ ] 
  [ ]  **`verifyTransaction` `startToCloseTimeout` (30s) vs. Tier-3 scan duration.** One attempt can run Tier-1 + Tier-2 (REST) + Tier-3 (up to 30 sequential `comet.block` round-trips). A slow attempt is killed with a `START_TO_CLOSE` `TimeoutFailure`, whose cause is **not** the `TX_NOT_FOUND` `ApplicationFailure` that `isTxNotFoundFailure()` checks for. On the final attempt this flips `verifyErroredUnexpectedly = true`, so a merely-not-yet-landed tx is logged as `verification errored (not a clean not-found) ... marked failure for triage` — a misleading triage signal. The timeout also consumes a `maximumAttempts` slot, shrinking the real verify budget below 30 blocks.
  [ ] 
  [ ]  **Tier-3 re-scans from scratch on every retry.** For a never-landing tx, ~30 retries × up to 30 sequential `comet.block(h)` fetches ≈ ~900 block RPC round-trips against one endpoint over ~30 min, plus a fresh client connection per attempt, mostly re-reading already-scanned blocks. Parallelize the block fetches (`Promise.all`) and/or only scan newly-produced blocks since the previous attempt.

Notes

The block-count↔retry-count coupling (maximumAttempts = TX_EXPIRATION_BLOCKS with a 60s interval, assuming ~60s block time) is a related fragility: if block time drifts, the verify window no longer matches the on-chain mempool-expiration window. Worth considering a height-based loop instead of a time-based proxy.

@jorgecuesta I commited some changes for these 3 robustness issues you reported #306 , here is the status for each one on the issues

For first issue
Proposed: bounded concurrency, like the SupplierStatus pattern.

Done as proposed: ExecutePendingTransactions now schedules children with pLimit + Promise.allSettled instead of a serial await loop, so one slow tx no longer blocks the rest.

For second issue
Proposed: classify a START_TO_CLOSE verify timeout as not-found instead of an unexpected error.

Done as suggested but with a small change: I made it its own "inconclusive" category (a verifyTimedOut flag + distinct log line) rather than folding it into not-found. Same outcome as wanted (no longer flagged as an error), but a timed-out scan stays distinguishable from a genuinely-absent tx for triage because merging them would have erased that signal.
Not addressed (sub-point: timeouts still consume a retry slot. A timed-out attempt uses one of the 30 retries; removing that would mean dropping Temporal's retry-policy model for verify.

For third issue
Proposed: parallelize the block fetches or/and scan only newly-produced blocks.
Done: took the parallelize option. getTransactionFromBlock now fetches candidate blocks in parallel via a bounded worker pool, plus a head-cap so the common case only scans the handful of blocks that exist.
Not done: "scan only new blocks" it needs to remember the last-scanned height between attempts, but our verify retries are stateless Temporal activity attempts and there's no shared cache. Doing it safely means moving the retry loop into the durable workflow (a refactor of the verify path)

Let me know what you think and if anything else should be changed, thanks!

[jorgecuesta]

but our verify retries are stateless Temporal activity attempts and there's no shared cache

@miguel502 so far all is in good shape for me we can keep improving the system without needs to be perfect in one shot. Next time think that you can use ContinueAsNew to trigger another workflow so you delivery it with extra data/state so it can continue from where you are, at least that work previously to me.