Priority: High
Description
NonceManager documents itself as serializing reservation per-account via this.locks, and reserve() correctly waits on/sets the lock. refresh() — meant to force a fresh read "after fee-bump or manual tx" — never touches this.locks and can run concurrently with an in-flight reserve() for the same account, corrupting the cache.
Location
engine-bridge/src/nonce-manager.ts:49-53 (refresh), racing with :24-39 (reserve)
Current Behavior
async refresh(accountId: string): Promise<void> {
this.cache.delete(accountId);
await this.nextSequence(accountId); // no lock at all
}
Expected Behavior
refresh() should participate in the same per-account lock as reserve() so the two can never interleave.
Repro / Evidence
reserve("G...") enters nextSequence, suspends on network I/O, holds no cache entry yet.
refresh("G...") runs concurrently, does its own getAccount round-trip, writes cache.
- Original
reserve()'s call resolves and unconditionally overwrites the cache, discarding refresh()'s value and returning a stale/colliding sequence.
Impact
Reopens exactly the sequence-collision hazard NonceManager exists to prevent, specifically on the recovery path an operator invokes when something has already gone wrong — compounding an incident.
Suggested Fix
Route refresh() through the same lock acquisition as reserve() (factor lock wait/acquire/release into a shared private helper).
Acceptance Criteria
Definition of Done
Priority: High
Description
NonceManagerdocuments itself as serializing reservation per-account viathis.locks, andreserve()correctly waits on/sets the lock.refresh()— meant to force a fresh read "after fee-bump or manual tx" — never touchesthis.locksand can run concurrently with an in-flightreserve()for the same account, corrupting the cache.Location
engine-bridge/src/nonce-manager.ts:49-53(refresh), racing with:24-39(reserve)Current Behavior
Expected Behavior
refresh()should participate in the same per-account lock asreserve()so the two can never interleave.Repro / Evidence
reserve("G...")entersnextSequence, suspends on network I/O, holds no cache entry yet.refresh("G...")runs concurrently, does its owngetAccountround-trip, writes cache.reserve()'s call resolves and unconditionally overwrites the cache, discardingrefresh()'s value and returning a stale/colliding sequence.Impact
Reopens exactly the sequence-collision hazard
NonceManagerexists to prevent, specifically on the recovery path an operator invokes when something has already gone wrong — compounding an incident.Suggested Fix
Route
refresh()through the same lock acquisition asreserve()(factor lock wait/acquire/release into a shared private helper).Acceptance Criteria
refresh()acquires the per-account lock before mutating cache.reserve()/refresh()for the same account can't interleave.getAccountduringreserve(), callingrefresh()mid-flight — fails on current code.Definition of Done