diff --git a/rollup.config.mjs b/rollup.config.mjs index 42210a5..4532e7f 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -76,7 +76,6 @@ const sharedOutput = { 'src/constants.ts', 'src/indexer.ts', 'src/fee-estimator.ts', - 'src/nonce/NonceManager.ts', 'src/relayer/WebSocketRelayer.ts', 'src/relayer/ErrorMapper.ts', 'src/adapters/index.ts', diff --git a/src/index.ts b/src/index.ts index 0600c69..05352c6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,8 +52,6 @@ export type { FeeEstimateOptions } from './fee-estimator.js'; export { WebSocketRelayer } from './relayer/WebSocketRelayer.js'; export { ErrorMapper } from './relayer/ErrorMapper.js'; export type { MappedErrorHandler } from './relayer/ErrorMapper.js'; -export { NonceManager } from './nonce/NonceManager.js'; -export type { NonceLock, NonceManagerOptions } from './nonce/NonceManager.js'; // Utils are exported via the /utils subpath export, but also available here export { diff --git a/src/nonce/NonceManager.ts b/src/nonce/NonceManager.ts deleted file mode 100644 index 7a6df16..0000000 --- a/src/nonce/NonceManager.ts +++ /dev/null @@ -1,300 +0,0 @@ -export interface NonceLock { - nonce: bigint; - release: () => void; -} - -export interface NonceManagerOptions { - startNonce?: bigint | number | string; - maxNonce?: bigint | number | string; -} - -const MAX_SAFE_U64 = 18446744073709551615n; - -interface QueueEntry { - resolve: (value: NonceLock) => void; - reject: (error: Error) => void; - cancelled: boolean; -} - -export class NonceManager { - private currentNonce: bigint; - private maxNonce: bigint; - private isLocked = false; - private lockQueue: QueueEntry[] = []; - private acquiredNonces: Set = new Set(); - private isDestroyed = false; - - constructor(options: NonceManagerOptions = {}) { - const rawStart = options.startNonce ?? 0n; - const rawMax = options.maxNonce ?? MAX_SAFE_U64; - - this.currentNonce = this.toSafeBigInt(rawStart); - this.maxNonce = this.toSafeBigInt(rawMax); - - if (this.currentNonce < 0n) { - throw new Error('NonceManager: startNonce cannot be negative'); - } - if (this.maxNonce <= this.currentNonce) { - throw new Error('NonceManager: maxNonce must be greater than startNonce'); - } - if (this.maxNonce > MAX_SAFE_U64) { - this.maxNonce = MAX_SAFE_U64; - } - } - - private toSafeBigInt(value: bigint | number | string): bigint { - if (typeof value === 'string' && value.trim() === '') { - // BigInt('') would coerce to 0n, silently masking a caller bug. - throw new Error('NonceManager: nonce value cannot be an empty string'); - } - try { - return BigInt(value); - } catch { - throw new Error( - `NonceManager: invalid nonce value "${String(value)}" — expected a non-negative integer (bigint, number, or numeric string)`, - ); - } - } - - private nonceKey(nonce: bigint): string { - return nonce.toString(); - } - - async acquire(): Promise { - if (this.isDestroyed) { - throw new Error('NonceManager has been destroyed'); - } - - if (!this.isLocked) { - this.isLocked = true; - return this.nextNonce(); - } - - return this.enqueue().promise; - } - - /** - * Queues a waiter for the lock and returns a handle that can cancel it. - * A cancelled waiter is skipped (without consuming a nonce) once it - * reaches the front of the queue, so an abandoned caller (e.g. a timed - * out `acquireWithFallback`) never leaves the lock permanently held. - */ - private enqueue(): { promise: Promise; cancel: () => void } { - const entry: QueueEntry = { resolve: () => undefined, reject: () => undefined, cancelled: false }; - const promise = new Promise((resolve, reject) => { - entry.resolve = resolve; - entry.reject = reject; - }); - this.lockQueue.push(entry); - return { promise, cancel: () => { entry.cancelled = true; } }; - } - - private nextNonce(): NonceLock { - const nonce = this.currentNonce; - if (nonce >= this.maxNonce) { - this.releaseLock(); - throw new Error(`NonceManager: nonce ${nonce} exceeds maximum ${this.maxNonce}`); - } - - this.currentNonce = nonce + 1n; - const key = this.nonceKey(nonce); - this.acquiredNonces.add(key); - - const release = () => { - this.acquiredNonces.delete(key); - this.releaseLock(); - }; - - return { nonce, release }; - } - - private releaseLock(): void { - let next = this.lockQueue.shift(); - while (next && next.cancelled) { - next = this.lockQueue.shift(); - } - if (next) { - try { - const lock = this.nextNonce(); - next.resolve(lock); - } catch (err) { - this.isLocked = false; - next.reject(err instanceof Error ? err : new Error(String(err))); - } - } else { - this.isLocked = false; - } - } - - get current(): bigint { - return this.currentNonce; - } - - get remaining(): bigint { - return this.maxNonce - this.currentNonce; - } - - get acquired(): number { - return this.acquiredNonces.size; - } - - async reset(nonce?: bigint | number): Promise { - if (this.isDestroyed) { - throw new Error('NonceManager has been destroyed'); - } - - const start = Date.now(); - while (this.isLocked && Date.now() - start < 5000) { - await new Promise(resolve => setTimeout(resolve, 10)); - } - - this.currentNonce = nonce !== undefined ? this.toSafeBigInt(nonce) : 0n; - this.acquiredNonces.clear(); - - const resetError = new Error('NonceManager reset'); - for (const entry of this.lockQueue) { - entry.reject(resetError); - } - this.lockQueue = []; - this.isLocked = false; - } - - destroy(): void { - this.isDestroyed = true; - - const destroyError = new Error('NonceManager destroyed'); - for (const entry of this.lockQueue) { - entry.reject(destroyError); - } - this.lockQueue = []; - this.isLocked = false; - this.acquiredNonces.clear(); - } - - static isNonceValid(value: unknown): value is bigint { - if (typeof value !== 'bigint' && typeof value !== 'number' && typeof value !== 'string') { - return false; - } - try { - const n = typeof value === 'bigint' ? value : BigInt(value); - return n >= 0n && n <= MAX_SAFE_U64; - } catch { - return false; - } - } - - async acquireWithFallback(timeoutMs = 5000): Promise { - if (this.isDestroyed) { - throw new Error('NonceManager has been destroyed'); - } - - if (!this.isLocked) { - this.isLocked = true; - return this.nextNonce(); - } - - const { promise, cancel } = this.enqueue(); - - let timer: ReturnType | undefined; - const timeoutPromise = new Promise((_, reject) => { - timer = setTimeout(() => { - cancel(); - reject(new Error(`NonceManager: acquire timed out after ${timeoutMs}ms`)); - }, timeoutMs); - }); - - try { - return await Promise.race([promise, timeoutPromise]); - } catch (err) { - if (err instanceof Error) { - throw err; - } - throw new Error(String(err)); - } finally { - if (timer) clearTimeout(timer); - } - } - - /** - * Safe acquisition with bounded waiting and exponential backoff between - * patience windows. - * - * Unlike a naive retry over {@link acquireWithFallback} — which `enqueue()`s - * a fresh waiter on every attempt, leaving cancelled entries in `lockQueue` - * and sending a retrying caller to the *back* of the line each time, so a - * caller that keeps just missing the window can be starved (#572) — this - * enqueues **exactly one** waiter. That waiter keeps its queue position - * across every attempt; a timed-out attempt just extends how long we wait - * on the same slot. The waiter is only cancelled once every retry is - * exhausted. - * - * @param retries number of patience windows - * @param delayMs base backoff between windows (× 2^attempt) - * @param perAttemptTimeoutMs how long each window waits before backing off - */ - async safeAcquire( - retries = 3, - delayMs = 100, - perAttemptTimeoutMs = 5000, - ): Promise { - if (this.isDestroyed) { - throw new Error('NonceManager has been destroyed'); - } - - // Fast path — the lock is free right now. - if (!this.isLocked) { - this.isLocked = true; - return this.nextNonce(); - } - - const { promise, cancel } = this.enqueue(); - - // If the lock frees while we are between attempts (during a backoff - // sleep), `releaseLock` resolves `promise` with the nonce even though - // nothing is awaiting it at that instant. Record it so we hand it back - // instead of cancelling — cancelling then would leak the held lock. - let handedLock: NonceLock | null = null; - void promise.then( - (lock) => { - handedLock = lock; - }, - () => undefined, - ); - - let lastError: Error | null = null; - - for (let attempt = 0; attempt < retries; attempt++) { - let timer: ReturnType | undefined; - const timeoutPromise = new Promise((_, reject) => { - timer = setTimeout( - () => - reject( - new Error( - `NonceManager: acquire attempt ${attempt + 1}/${retries} timed out after ${perAttemptTimeoutMs}ms`, - ), - ), - perAttemptTimeoutMs, - ); - }); - - try { - const lock = await Promise.race([promise, timeoutPromise]); - if (timer) clearTimeout(timer); - return lock; - } catch (err) { - if (timer) clearTimeout(timer); - lastError = err instanceof Error ? err : new Error(String(err)); - // The single waiter is still queued (not cancelled) — just wait a - // bit longer on the same slot. - if (attempt < retries - 1) { - await new Promise((r) => setTimeout(r, delayMs * 2 ** attempt)); - if (handedLock) return handedLock; - } - } - } - - if (handedLock) return handedLock; - cancel(); - throw lastError ?? new Error('NonceManager: safeAcquire failed'); - } -} diff --git a/src/tests/bug-fixes-577-572.test.ts b/src/tests/bug-fixes-577-572.test.ts index 7b4a728..118c69e 100644 --- a/src/tests/bug-fixes-577-572.test.ts +++ b/src/tests/bug-fixes-577-572.test.ts @@ -1,15 +1,12 @@ /** - * Regression tests for issues #577 and #572. + * Regression tests for issue #577. * * - #577 `u64ToScVal` / `estimateRequiredFee` — no raw `RangeError` from an * unguarded `BigInt(x)` on a float or negative value. - * - #572 `NonceManager.safeAcquire` — one queued waiter across retries, so a - * retrying caller is not sent to the back of the line and starved. */ import { describe, it, expect } from 'vitest'; import { u64ToScVal, estimateRequiredFee } from '../soroban.js'; -import { NonceManager, type NonceLock } from '../nonce/NonceManager.js'; // ── #577: u64ToScVal ───────────────────────────────────────────────────────── @@ -49,69 +46,3 @@ describe('#577 — estimateRequiredFee never throws on a non-conforming fee fiel expect(estimateRequiredFee({ minResourceFee: 1000n })).toBe(1000n); }); }); - -// ── #572: NonceManager.safeAcquire ─────────────────────────────────────────── - -describe('#572 — NonceManager.safeAcquire uses one waiter and does not starve a retrying caller', () => { - it('acquires the lock immediately when it is free', async () => { - const m = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - const lock = await m.safeAcquire(); - expect(lock.nonce).toBe(0n); - lock.release(); - m.destroy(); - }); - - it('a caller waiting through several patience windows still gets the next nonce (bounded wait, not back-of-line)', async () => { - const m = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - - const held = await m.acquire(); // nonce 0, lock held - - // Short windows so safeAcquire "retries" a few times while blocked. - const waiter = m.safeAcquire(4, 5, 20); - // A later arrival that queues *after* the retrying caller. - await new Promise((r) => setTimeout(r, 35)); - const late = m.acquire(); - - // Free the lock — the earliest queued waiter (safeAcquire's single entry) - // must be served first, not the later arrival. - held.release(); - - const first = await waiter; - expect(first.nonce).toBe(1n); - first.release(); - - const l = await late; - expect(l.nonce).toBe(2n); - l.release(); - m.destroy(); - }); - - it('does not leave the lock permanently held when safeAcquire gives up', async () => { - const m = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - const held = await m.acquire(); - - await expect(m.safeAcquire(2, 1, 10)).rejects.toThrow(/timed out/); - - // The abandoned waiter must not block the next real acquirer. - held.release(); - const next = await m.acquire(); - expect(next.nonce).toBe(1n); - next.release(); - m.destroy(); - }); - - it('concurrent safeAcquire callers all get distinct nonces', async () => { - const m = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - const locks: NonceLock[] = await Promise.all( - Array.from({ length: 8 }, () => - m.safeAcquire(5, 2, 200).then((l) => { - setTimeout(() => l.release(), 0); - return l; - }), - ), - ); - const nonces = locks.map((l) => l.nonce.toString()).sort(); - expect(new Set(nonces).size).toBe(8); - m.destroy(); - }); -}); diff --git a/src/tests/nonce-concurrent.test.ts b/src/tests/nonce-concurrent.test.ts deleted file mode 100644 index 58439c4..0000000 --- a/src/tests/nonce-concurrent.test.ts +++ /dev/null @@ -1,189 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { NonceManager, type NonceLock } from '../nonce/NonceManager.js'; - -describe('NonceManager — Concurrent Nonce Integration Tests', () => { - let manager: NonceManager; - - beforeEach(() => { - manager = new NonceManager({ startNonce: 0n, maxNonce: 1000n }); - }); - - afterEach(() => { - manager.destroy(); - }); - - it('acquires sequential nonces without gaps', async () => { - const locks: NonceLock[] = []; - for (let i = 0; i < 5; i++) { - const lock = await manager.acquire(); - expect(lock.nonce).toBe(BigInt(i)); - locks.push(lock); - lock.release(); - } - - for (const lock of locks) { - lock.release(); - } - }); - - it('does not duplicate nonces under concurrent load', async () => { - const count = 100; - const nonces: bigint[] = []; - - for (let i = 0; i < count; i++) { - const lock = await manager.acquire(); - nonces.push(lock.nonce); - lock.release(); - } - - const uniqueNonces = new Set(nonces.map(n => n.toString())); - expect(uniqueNonces.size).toBe(count); - expect(nonces).toEqual(Array.from({ length: count }, (_, i) => BigInt(i))); - }); - - it('tracks acquired nonces correctly', async () => { - const lock1 = await manager.acquire(); - expect(manager.acquired).toBe(1); - lock1.release(); - expect(manager.acquired).toBe(0); - - const lock2 = await manager.acquire(); - expect(manager.acquired).toBe(1); - lock2.release(); - expect(manager.acquired).toBe(0); - }); - - it('respects maxNonce boundary', async () => { - const small = new NonceManager({ startNonce: 0n, maxNonce: 5n }); - const locks = []; - - for (let i = 0; i < 5; i++) { - const lock = await small.acquire(); - locks.push(lock); - lock.release(); - } - - expect(small.remaining).toBe(0n); - await expect(small.acquire()).rejects.toThrow('exceeds maximum'); - - small.destroy(); - }); - - it('acquireWithFallback works with proper release pattern', async () => { - const m = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - - const lock1 = await m.acquire(); - expect(lock1.nonce).toBe(0n); - lock1.release(); - - const lock2 = await m.acquireWithFallback(100); - expect(lock2.nonce).toBe(1n); - lock2.release(); - - m.destroy(); - }); - - it('does not deadlock after acquireWithFallback times out on a held lock', async () => { - const m = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - - const lock1 = await m.acquire(); - expect(lock1.nonce).toBe(0n); - - await expect(m.acquireWithFallback(20)).rejects.toThrow('timed out'); - - lock1.release(); - - const lock2 = await m.acquireWithFallback(50); - expect(lock2.nonce).toBe(1n); - lock2.release(); - - m.destroy(); - }); - - it('safeAcquire retries on failure', async () => { - const tiny = new NonceManager({ startNonce: 0n, maxNonce: 1n }); - - const lock = await tiny.acquire(); - lock.release(); - expect(tiny.remaining).toBe(0n); - - await expect(tiny.safeAcquire(2, 10)).rejects.toThrow(); - - tiny.destroy(); - }); - - it('validates nonce values correctly', () => { - expect(NonceManager.isNonceValid(0n)).toBe(true); - expect(NonceManager.isNonceValid(100n)).toBe(true); - expect(NonceManager.isNonceValid('12345')).toBe(true); - expect(NonceManager.isNonceValid(0)).toBe(true); - expect(NonceManager.isNonceValid(-1n)).toBe(false); - expect(NonceManager.isNonceValid(null)).toBe(false); - expect(NonceManager.isNonceValid(undefined)).toBe(false); - expect(NonceManager.isNonceValid('abc')).toBe(false); - expect(NonceManager.isNonceValid({})).toBe(false); - }); - - it('rejects negative startNonce', () => { - expect(() => new NonceManager({ startNonce: -1n })).toThrow( - 'startNonce cannot be negative', - ); - }); - - it('rejects maxNonce <= startNonce', () => { - expect(() => new NonceManager({ startNonce: 10n, maxNonce: 10n })).toThrow( - 'maxNonce must be greater than startNonce', - ); - }); - - it('converts string nonces to bigint', () => { - const m = new NonceManager({ startNonce: '42', maxNonce: '100' }); - expect(m.current).toBe(42n); - m.destroy(); - }); - - it('handles string bigint nonces safely', () => { - const m = new NonceManager({ startNonce: '9007199254740993', maxNonce: '9007199254740994' }); - expect(m.current).toBe(9007199254740993n); - m.destroy(); - }); - - it('throws a descriptive error for an unparseable string startNonce', () => { - // Regression test for #458: an unparseable nonce must not silently - // coerce to 0n, which would mask a caller bug (e.g. a stringified - // undefined or a malformed network response) as an explicit 0. - expect(() => new NonceManager({ startNonce: 'not-a-number' })).toThrow( - /invalid nonce value "not-a-number"/, - ); - }); - - it('throws for an empty-string startNonce instead of silently using 0n', () => { - expect(() => new NonceManager({ startNonce: '', maxNonce: '100' })).toThrow( - /empty string/, - ); - }); - - it('accepts numeric and bigint startNonce values', () => { - const fromNumber = new NonceManager({ startNonce: 42, maxNonce: 100 }); - expect(fromNumber.current).toBe(42n); - fromNumber.destroy(); - - const fromBigInt = new NonceManager({ startNonce: 7n, maxNonce: 100n }); - expect(fromBigInt.current).toBe(7n); - fromBigInt.destroy(); - }); - - it('reset clears state and allows reacquisition', async () => { - const lock1 = await manager.acquire(); - lock1.release(); - expect(manager.current).toBe(1n); - - await manager.reset(0n); - expect(manager.current).toBe(0n); - expect(manager.acquired).toBe(0); - - const lock3 = await manager.acquire(); - expect(lock3.nonce).toBe(0n); - lock3.release(); - }); -}); \ No newline at end of file diff --git a/src/tests/nonce-manager-export.test.ts b/src/tests/nonce-manager-export.test.ts deleted file mode 100644 index 87fe27b..0000000 --- a/src/tests/nonce-manager-export.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { describe, expect, it } from 'vitest'; - -describe('NonceManager public export', () => { - it('is exported from the package entry point as the bigint-based implementation', async () => { - const { NonceManager } = await import('../index.js'); - - const manager = new NonceManager({ startNonce: 0n, maxNonce: 10n }); - const lock = await manager.acquire(); - - // The bigint-based `src/nonce/NonceManager.ts` implementation hands out - // `bigint` nonces; the number-based `src/nonce-manager.ts` duplicate - // (which cannot represent Stellar int64 sequence numbers above 2^53) - // does not expose `acquire()`/`release()` at all. - expect(typeof lock.nonce).toBe('bigint'); - expect(lock.nonce).toBe(0n); - - lock.release(); - manager.destroy(); - }); -}); diff --git a/src/tests/nonce-safe-acquire-backoff.test.ts b/src/tests/nonce-safe-acquire-backoff.test.ts deleted file mode 100644 index eabaa85..0000000 --- a/src/tests/nonce-safe-acquire-backoff.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { NonceManager } from '../nonce/NonceManager.js'; - -describe('NonceManager.safeAcquire exponential backoff', () => { - afterEach(() => { - vi.useRealTimers(); - vi.restoreAllMocks(); - }); - - it('doubles the delay between bounded waiting windows', async () => { - vi.useFakeTimers(); - - const manager = new NonceManager({ startNonce: 0n, maxNonce: 100n }); - const heldLock = await manager.acquire(); - - try { - const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout'); - const acquisition = manager.safeAcquire(4, 100, 10); - const rejection = expect(acquisition).rejects.toThrow( - 'acquire attempt 4/4 timed out after 10ms', - ); - - await vi.runAllTimersAsync(); - await rejection; - - expect(setTimeoutSpy.mock.calls.map(([, delay]) => delay)).toEqual([ - 10, - 100, - 10, - 200, - 10, - 400, - 10, - ]); - } finally { - heldLock.release(); - manager.destroy(); - } - }); -});