Skip to content

[BUG] TxAggregator.execute() leaks reserved nonces on failed/timed-out submissions, permanently desyncing the account's sequence cache #181

Description

@N-thnI

Priority: High

Description

TxAggregator.execute() never releases the nonce reserved in build() when submission fails before reaching the ledger — either sendTransaction returning status: "ERROR" (sequence never consumed on-chain) or the poll loop timing out. execute() doesn't even receive sourceAccountId/sequence, so there's no way for it to call nonceManager.release(...) on these paths.

Location

engine-bridge/src/tx-aggregator.ts:123-157 (execute)

Current Behavior

async execute(transaction: Transaction, opts: {...} = {}): Promise<...> {
  const sendResponse = await this.rpc.call(async (server) => server.sendTransaction(transaction));
  if (sendResponse.status === "ERROR") {
    throw new Error(`TxAggregator: sendTransaction failed with status ERROR: ...`);
  }
  while (true) {
    if (Date.now() - startTime > timeoutMs) {
      throw new Error(`TxAggregator: transaction execution timed out after ${timeoutMs}ms`);
    }
    ...

Expected Behavior

On a submission failure known not to have consumed the on-chain sequence, the reserved sequence should be released back to NonceManager.

Repro / Evidence

const nm = new NonceManager(rpc);
const aggregator = new TxAggregator(rpc, nm, new GasOracle(), Networks.TESTNET);
const batch = await aggregator.build({ ... });
await expect(aggregator.execute(batch.transaction, {...})).rejects.toThrow();
const nextSeq = await nm.reserve(signer.publicKey());
expect(nextSeq).toBe(batch.sequence); // FAILS -- nextSeq is batch.sequence + 1

Impact

Every failed/rejected/timed-out submission permanently burns a sequence number in the in-process cache without touching the chain. Under elevated error rates, the cache drifts ahead of the real account sequence until all transactions from that wallet fail with tx_bad_seq — a full outage of the fund-moving path requiring manual refresh().

Suggested Fix

Thread sourceAccountId/sequence into execute() and call nonceManager.release(...) in the ERROR and timeout catch paths.

Acceptance Criteria

  • AC-1: execute() releases the sequence on sendTransaction ERROR.
  • AC-2: execute() releases the sequence on poll-loop timeout.
  • AC-3: New test reproducing the leak — fails on current code, passes after fix.

Definition of Done

  • Fix merged with all AC items checked
  • Regression test passes in CI
  • No new eslint warnings

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaignbugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions