Skip to content

Distributed lock TTL expires during long Stellar operations FIXED#55

Merged
DeFiVC merged 2 commits into
ChainLearnOfficial:mainfrom
Mitch5000:Distributed-lock-TTL-expires-during-long-Stellar-operations--FIXED
Jun 27, 2026
Merged

Distributed lock TTL expires during long Stellar operations FIXED#55
DeFiVC merged 2 commits into
ChainLearnOfficial:mainfrom
Mitch5000:Distributed-lock-TTL-expires-during-long-Stellar-operations--FIXED

Conversation

@Mitch5000

Copy link
Copy Markdown
Contributor

Pull Request Title
fix: implement distributed lock renewal heartbeat and increase default TTL

Pull Request Description
Issue addressed:
The distributed lock in
lock.ts
used a fixed 10s TTL. Stellar contract invocations (involving Horizon load, simulation, and submission) frequently exceed this duration, leading to premature lock release and potential double-claim/double-mint vulnerabilities.

Changes implemented:

Lock Renewal (Heartbeat) Mechanism: Added a heartbeat in withLock that automatically extends the Redis lock TTL every ttlMs / 2 while the task is still executing.
Increased Default TTL: Raised the default TTL from 10s to 30s to provide a more robust baseline for Stellar operations.
Atomic Renewal via Lua: Used an atomic Lua script for renewal to ensure only the current lock holder (validated by a unique UUID) can extend the lock.
Resource Cleanup: Guaranteed that the heartbeat interval is cleared in the finally block to prevent memory leaks and ensure immediate lock release after task completion.
New Unit Tests: Added comprehensive tests in
lock.test.ts
to verify:
Successful lock acquisition and release.
Conflict handling when a lock is already held.
Automatic heartbeat renewal during long-running tasks.
Heartbeat termination if the lock is lost or the task finishes.
Files modified/created:

(src/utils/lock.ts)
(tests/unit/utils/lock.test.ts)
Impact:
This fix restores mutual exclusion for critical operations (reward claiming and credential minting) regardless of external API latency, effectively preventing race conditions and double-minting.

How to apply:
If you are working in this environment, the changes are already committed to the local branch fix/distributed-lock-renewal. You can view the changes with:

Bash

git diff main fix/distributed-lock-renewal

Closes #27

@DeFiVC DeFiVC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: fix: implement distributed lock renewal heartbeat and increase default TTL

Author: Mitch5000 | Closes: #27 | CI: ⚠️ action_required (repo approval needed, not a code issue) | Conflicts: ✅ None

Blocking Issues

None. The CI action_required status is a repository configuration issue (manual workflow approval required for this contributor) — not caused by this PR's code.

Code Issues

1. Lost lock doesn't abort executionsrc/utils/lock.ts:35-40

When the heartbeat detects the lock was lost (result !== 1), it clears itself and logs a warning, but the function continues executing. This means another process could acquire the lock while this one is still running, defeating mutual exclusion. Consider:

  • Rejecting the in-progress fn() call, or
  • At minimum, documenting this as an accepted trade-off for graceful degradation

This is a design choice rather than a bug — the heartbeat primarily fixes the premature expiry problem (the actual issue). Lost-lock detection is a separate concern.

Minor Nits

  • setInterval with async callbacksrc/utils/lock.ts:25-48: If redis.eval takes longer than ttlMs / 2 (15s), multiple renewal callbacks could queue. In practice Redis evals complete in <100ms so this is unlikely, but an alternative is setTimeout + recursive scheduling to guarantee serial execution.

What's Good

  • Core fix is correct: TTL increased from 10s → 30s, heartbeat at 15s provides a 15s safety margin before expiry
  • Atomic Lua renewal: Validates lock ownership before extending — prevents extending a lock that was already taken by another holder
  • Proper cleanup: finally block always clears the heartbeat interval, preventing memory leaks
  • Heartbeat self-terminates: Clears itself when renewal fails (lock lost) or when fn() completes — no orphaned timers
  • Comprehensive tests: Covers acquisition, conflict, heartbeat renewal, and heartbeat termination on failure. Uses vi.useFakeTimers() correctly for deterministic timer testing
  • Minimal scope: Only 2 files changed, focused fix with no scope creep

The heartbeat mechanism correctly addresses the root cause described in #27 — premature lock release during long Stellar operations. Approving."

@DeFiVC DeFiVC merged commit 7a7a817 into ChainLearnOfficial:main Jun 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[HIGH] Distributed lock TTL expires during long Stellar operations

2 participants