📌 Description
internal/soroban/rpc.go's pollTransactionStatusOnce and internal/soroban/tx.go's WaitForConfirmation both implement the same polling pattern:
deadline := time.Now().Add(timeout)
ticker := time.NewTicker(2 * time.Second)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-ticker.C:
if time.Now().After(deadline) {
return nil, fmt.Errorf("timeout waiting for transaction...")
}
// ... do one poll attempt ...
}
}Because the deadline is only ever checked inside the <-ticker.C branch, and time.NewTicker(2 * time.Second) doesn't fire until 2 seconds have elapsed, a caller that passes a timeout shorter than 2 seconds (e.g. timeout: 500 * time.Millisecond in a fast unit/integration test, or a deliberately tight caller-side budget) never gets a timeout error at all within the requested window — the function keeps waiting for the first tick, checks the deadline, finds it already passed, and only then returns the timeout error, roughly 1.5+ seconds later than requested. internal/soroban/rpc.go has no test file today, so this has never been exercised directly (only indirectly through higher-level Soroban tests that use longer timeouts).
🧩 Requirements and context
- Both polling loops should honor
timeout promptly regardless of how short it is relative to the fixed 2-second poll interval — the returned timeout error should not be reliably delayed by up to one extra poll interval beyond the requested deadline.
- Prefer deriving a
context.WithTimeout(ctx, timeout) internally (or wiring the existing ticker against a proper deadline-aware timer) rather than only checking wall-clock time inside the ticker branch.
- Preserve existing behavior otherwise: still poll roughly every 2 seconds, still return
ctx.Err() promptly if the caller's own context is cancelled independent of timeout.
🛠️ Suggested execution
- In
pollTransactionStatusOnce (internal/soroban/rpc.go) and WaitForConfirmation (internal/soroban/tx.go), replace the manual deadline/time.Now().After(deadline) check with a derived ctx, cancel := context.WithTimeout(ctx, timeout); defer cancel(), and add a case <-ctx.Done(): return nil, timeoutOrCtxErr branch that distinguishes "the derived timeout fired" from "the caller's original context was cancelled" if that distinction matters to callers (check current callers' error-handling expectations before deciding).
- Add
internal/soroban/rpc_test.go (does not exist today) with a test asserting that a timeout shorter than the 2-second poll interval (e.g. 300 * time.Millisecond) causes pollTransactionStatusOnce/PollTransactionStatus to return promptly (within a small margin of timeout, not ~2 seconds later).
- Add the equivalent short-timeout test for
WaitForConfirmation if internal/soroban/tx_test.go exists or is being added as part of related work.
✅ Acceptance criteria
🔒 Security notes
No new attack surface; this is a correctness fix for timeout handling that could otherwise cause callers (e.g. request handlers with their own upstream deadlines) to block longer than intended waiting on a slow/unresponsive Soroban RPC or Horizon endpoint.
📋 Guidelines
- Minimum 95% test coverage
- Clear documentation
- Timeframe: 96 hours
📌 Description
internal/soroban/rpc.go'spollTransactionStatusOnceandinternal/soroban/tx.go'sWaitForConfirmationboth implement the same polling pattern:Because the deadline is only ever checked inside the
<-ticker.Cbranch, andtime.NewTicker(2 * time.Second)doesn't fire until 2 seconds have elapsed, a caller that passes atimeoutshorter than 2 seconds (e.g.timeout: 500 * time.Millisecondin a fast unit/integration test, or a deliberately tight caller-side budget) never gets a timeout error at all within the requested window — the function keeps waiting for the first tick, checks the deadline, finds it already passed, and only then returns the timeout error, roughly 1.5+ seconds later than requested.internal/soroban/rpc.gohas no test file today, so this has never been exercised directly (only indirectly through higher-level Soroban tests that use longer timeouts).🧩 Requirements and context
timeoutpromptly regardless of how short it is relative to the fixed 2-second poll interval — the returned timeout error should not be reliably delayed by up to one extra poll interval beyond the requested deadline.context.WithTimeout(ctx, timeout)internally (or wiring the existing ticker against a proper deadline-aware timer) rather than only checking wall-clock time inside the ticker branch.ctx.Err()promptly if the caller's own context is cancelled independent oftimeout.🛠️ Suggested execution
pollTransactionStatusOnce(internal/soroban/rpc.go) andWaitForConfirmation(internal/soroban/tx.go), replace the manualdeadline/time.Now().After(deadline)check with a derivedctx, cancel := context.WithTimeout(ctx, timeout); defer cancel(), and add acase <-ctx.Done(): return nil, timeoutOrCtxErrbranch that distinguishes "the derived timeout fired" from "the caller's original context was cancelled" if that distinction matters to callers (check current callers' error-handling expectations before deciding).internal/soroban/rpc_test.go(does not exist today) with a test asserting that atimeoutshorter than the 2-second poll interval (e.g.300 * time.Millisecond) causespollTransactionStatusOnce/PollTransactionStatusto return promptly (within a small margin oftimeout, not ~2 seconds later).WaitForConfirmationifinternal/soroban/tx_test.goexists or is being added as part of related work.✅ Acceptance criteria
timeoutshorter than the 2-second poll interval causes a prompt timeout error, not one delayed until the next ticker tick.pollTransactionStatusOnce.🔒 Security notes
No new attack surface; this is a correctness fix for timeout handling that could otherwise cause callers (e.g. request handlers with their own upstream deadlines) to block longer than intended waiting on a slow/unresponsive Soroban RPC or Horizon endpoint.
📋 Guidelines