Fixed checkAllowance false-negative

app/create/__tests__/page.test.tsx

@@ -51,6 +51,15 @@ vi.mock('@/lib/token-allowance-gateway', () => ({
   }),
 }));
 
+const mockCheckRecipientExists = vi.fn().mockResolvedValue(true);
+vi.mock('@/lib/soroban', async () => {
+  const actual = await vi.importActual<typeof import('@/lib/soroban')>('@/lib/soroban');
+  return {
+    ...actual,
+    checkRecipientExists: (...args: unknown[]) => mockCheckRecipientExists(...args),
+  };
+});
+
 vi.mock('lucide-react', () => ({
   ArrowRight: () => React.createElement('span', null, '→'),
   Info: () => React.createElement('span', null, 'i'),
@@ -96,6 +105,11 @@ async function fillRecipient(container: HTMLElement) {
   await act(async () => {
     setFieldValue(recipientInput, TEST_RECIPIENT);
   });
+  // Recipient existence check is debounced 600ms + RPC; wait for it to settle
+  // so the form isn't blocked by `recipientStatus === 'checking'`.
+  await act(async () => {
+    await new Promise((r) => setTimeout(r, 700));
+  });
 }
 
 async function fillDeposit(container: HTMLElement, amount: string) {
@@ -110,6 +124,7 @@ async function fillDeposit(container: HTMLElement, amount: string) {
 describe('CreatePage — zero-rate guard (issue #243)', () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    mockCheckRecipientExists.mockResolvedValue(true);
     mockCreateStream.mockResolvedValue({ hash: 'tx_hash_abc', streamId: 7n });
     mockRefreshStreamData.mockResolvedValue(undefined);
   });
@@ -179,6 +194,7 @@ describe('CreatePage — SEP-41 allowance check before deposit (issue #218)', ()
 
   beforeEach(() => {
     vi.clearAllMocks();
+    mockCheckRecipientExists.mockResolvedValue(true);
     mockIsMock.mockReturnValue(false);
     mockCreateStream.mockResolvedValue({ hash: 'tx_hash_abc', streamId: 7n });
     mockRefreshStreamData.mockResolvedValue(undefined);

lib/soroban-pipeline.test.ts

@@ -82,7 +82,7 @@ function simError(message: string) {
   return { error: message };
 }
 
-beforeEach(() => {
+beforeEach(async () => {
   vi.useFakeTimers();
   mockGetAccount.mockReset().mockResolvedValue({ accountId: () => SOURCE, sequenceNumber: () => '1' });
   mockSimulate.mockReset();
@@ -93,6 +93,10 @@ beforeEach(() => {
   mockAssemble.mockReset().mockReturnValue({
     build: () => ({ toEnvelope: () => ({ toXDR: () => 'assembled-envelope-b64' }) }),
   });
+  // Clear the inclusion-fee cache so the "falls back to BASE_FEE" test
+  // doesn't see a cached p70 from the previous test's successful fetch.
+  const { __clearFeeStatsCache } = await import('./soroban.js');
+  __clearFeeStatsCache();
 });
 
 afterEach(() => {
@@ -212,7 +216,7 @@ describe('invokeContract', () => {
     promise.catch(() => {});
     await vi.advanceTimersByTimeAsync(2000);
 
-    expect(await promise).toBe('deadbeef');
+    expect(await promise).toEqual(expect.objectContaining({ hash: 'deadbeef' }));
     expect(signTx).toHaveBeenCalledTimes(1);
     expect(mockSend).toHaveBeenCalledTimes(1);
   });
@@ -227,7 +231,7 @@ describe('invokeContract', () => {
     promise.catch(() => {});
     await vi.advanceTimersByTimeAsync(31_000);
 
-    expect(await promise).toBe('deadbeef');
+    expect(await promise).toEqual(expect.objectContaining({ hash: 'deadbeef' }));
     expect(signTx).toHaveBeenCalledTimes(1);
     expect(mockSend).toHaveBeenCalledTimes(1);
   });

lib/soroban.ts

@@ -121,6 +121,12 @@ function getServer(): SorobanRpc.Server {
  */
 export function resetServer(): void {
   serverInstance = undefined;
+  feeStatsCache = undefined;
+}
+
+/** Test-only: clear the inclusion-fee cache so fee-stats tests are isolated. */
+export function __clearFeeStatsCache(): void {
+  feeStatsCache = undefined;
 }
 
 // ── Retry / Backoff ───────────────────────────────────────────────────────────
@@ -426,25 +432,6 @@ export async function invokeContract(
       throw new Error('Submission returned no transaction hash');
     }
 
-        if (status.status === SorobanRpc.Api.GetTransactionStatus.SUCCESS) {
-          // #362 — surface the confirmed transaction's return value instead
-          // of discarding it. Contract functions like DripFactory::create_stream
-          // return data (the assigned stream_id) that callers otherwise have
-          // no way to obtain without a separate re-query.
-          return { hash, returnValue: status.returnValue };
-        }
-        if (status.status === SorobanRpc.Api.GetTransactionStatus.FAILED) {
-          // Non-retryable — transaction executed and failed on-chain
-          recordFailure();
-          throw new Error(`Transaction failed: ${hash}`);
-        }
-        // status === 'NOT_FOUND' — keep polling
-      }
-      throw new Error(`Transaction timed out after ${MAX_POLL_ATTEMPTS}s: ${hash}`);
-    }, {
-      context: `invokeContract(${method})`,
-      signal,
-    });
     return pollForConfirmation(hash, timeoutMs, signal);
   };
 
@@ -461,12 +448,16 @@ export async function invokeContract(
  * reach a verdict (transient RPC errors, or the confirmation window elapsing)
  * the hash is returned as *pending* rather than replaying the transaction:
  * it is already on-chain and the caller can look it up (see #358).
+ *
+ * On SUCCESS the confirmed transaction's return value is surfaced so callers
+ * like DripFactory::create_stream can obtain the assigned stream_id without
+ * a separate re-query (see #362).
  */
 async function pollForConfirmation(
   hash: string,
   timeoutMs: number,
   signal?: AbortSignal,
-): Promise<string> {
+): Promise<InvokeContractResult> {
   for (let i = 0; i < MAX_POLL_ATTEMPTS; i++) {
     if (signal?.aborted) throw new OperationAbortedError();
 
@@ -492,7 +483,7 @@ async function pollForConfirmation(
     if (status.status === SorobanRpc.Api.GetTransactionStatus.SUCCESS) {
       // The RPC round-trip completed — the network is healthy.
       resetCircuitBreaker();
-      return hash;
+      return { hash, returnValue: status.returnValue };
     }
     if (status.status === SorobanRpc.Api.GetTransactionStatus.FAILED) {
       // The transaction executed and the contract reverted. The RPC worked
@@ -504,7 +495,7 @@ async function pollForConfirmation(
   }
 
   // Submitted but unconfirmed within the window — pending, not failed.
-  return hash;
+  return { hash };
 }
 
 /**

lib/stream.test.ts

@@ -116,7 +116,9 @@ describe('getStreamInfo', () => {
   it('throws a clear error when a field is missing rather than silently defaulting', async () => {
     mockSimulateReadOnly.mockResolvedValue(scvMap({
       sender: new Address(SENDER).toScVal(),
-      // recipient deliberately omitted
+      // recipient deliberately omitted — flags included so the missing-recipient
+      // path is exercised first (flags is now read before sender/recipient).
+      flags: xdr.ScVal.scvU32(0),
     }));
     const { getStreamInfo } = await import('./stream.js');
     await expect(getStreamInfo(SENDER, STREAM_ADDRESS)).rejects.toThrow(/Missing field: recipient/);

lib/tokens.test.ts

@@ -102,8 +102,21 @@ describe('SEP-41 Token Allowance Helpers (#347, #348)', () => {
     expect(result.currentAllowance).toBe(1000n);
   });
 
+  it('checkAllowance distinguishes RPC failure from insufficient allowance (hasAllowance undefined, not false)', async () => {
+    vi.mocked(soroban.simulateReadOnly).mockRejectedValueOnce(new Error('Network request timed out'));
+    const result = await checkAllowance(VALID_SOURCE, VALID_TOKEN, VALID_SPENDER, 500n);
+    // Must NOT be `false` — false means "checked, insufficient" and would
+    // trigger an unnecessary approve() and fee. Undefined means "couldn't check".
+    expect(result.hasAllowance).toBeUndefined();
+    expect(result.currentAllowance).toBe(0n);
+    expect(result.error).toMatch(/Network request timed out/);
+    // Callers must check error first; a falsy check `!hasAllowance` would
+    // still be true for undefined, so the correct guard is `hasAllowance === false`.
+    expect(result.hasAllowance === false).toBe(false);
+  });
+
   it('approveAllowance calls SAC with 4 arguments and succeeds (#347)', async () => {
-    vi.mocked(soroban.invokeContract).mockResolvedValueOnce('tx_hash_123');
+    vi.mocked(soroban.invokeContract).mockResolvedValueOnce({ hash: 'tx_hash_123' } as any);
     const mockSignTx = vi.fn().mockResolvedValue('signed');
 
     const result = await approveAllowance(
@@ -127,7 +140,7 @@ describe('SEP-41 Token Allowance Helpers (#347, #348)', () => {
     );
 
     // Verify 4 arguments were passed to invokeContract
-    const passedArgs = vi.mocked(soroban.invokeContract).mock.calls[0][3];
+    const passedArgs = vi.mocked(soroban.invokeContract).mock.calls[0]![3] as unknown[];
     expect(passedArgs).toHaveLength(4);
   });
 });

lib/tokens.ts

@@ -92,7 +92,16 @@ import {
 } from './token-allowance-gateway';
 
 export interface AllowanceResult {
-  hasAllowance: boolean;
+  /**
+   * Whether the spender has sufficient allowance.
+   * - `true`  → allowance >= requiredAmount (checked, sufficient)
+   * - `false` → allowance < requiredAmount (checked, insufficient — needs approve)
+   * - `undefined` → allowance could not be checked (e.g. transient RPC failure).
+   *   Callers MUST check `error` first and retry the read rather than treating
+   *   `undefined` as "needs approval", otherwise a hiccup triggers an
+   *   unnecessary SEP-41 approve() transaction and fee.
+   */
+  hasAllowance: boolean | undefined;
   currentAllowance: bigint;
   error?: string;
 }
@@ -139,6 +148,12 @@ export async function getAllowance(
 /**
  * Check whether the source address has sufficient token allowance for spender.
  * Throws an explicit error on missing arguments (#348), returns structured status on check completion.
+ *
+ * IMPORTANT: On a successful RPC read, `hasAllowance` is `true`|`false` and
+ * `error` is absent. On a transient RPC/network failure, `hasAllowance` is
+ * `undefined` and `error` is set — callers must surface the error and retry
+ * the read rather than treating it as "insufficient allowance" (which would
+ * trigger an unnecessary approve() transaction and fee).
  */
 export async function checkAllowance(
   source: string,
@@ -162,7 +177,7 @@ export async function checkAllowance(
   } catch (err: unknown) {
     const message = err instanceof Error ? err.message : 'Failed to check token allowance';
     return {
-      hasAllowance: false,
+      hasAllowance: undefined,
       currentAllowance: 0n,
       error: message,
     };