diff --git a/__tests__/integration/service-specific/start.test.ts b/__tests__/integration/service-specific/start.test.ts index 4e7744426..3dc749fa3 100644 --- a/__tests__/integration/service-specific/start.test.ts +++ b/__tests__/integration/service-specific/start.test.ts @@ -122,6 +122,24 @@ describe('[Service-specific] start', () => { expect(currentAddress.address).toEqual(emptyWallet.addresses[currentAddress.index]); }); + it('should complete start() on a brand-new wallet that goes through creating→ready', async () => { + // This test exercises the full auth flow: createWallet returns 'creating', + // then pollForWalletStatus polls until 'ready'. Auth tokens are obtained + // on-demand by the axios interceptor during each polling call. This is + // the exact path where the old fire-and-forget pattern raced. + ({ wallet } = buildWalletInstance()); + + await wallet.start({ pinCode, password }); + + expect(wallet.isReady()).toBe(true); + expect(wallet.getAuthToken()).not.toBeNull(); + + // Verify the wallet is functional by checking it has an address + const currentAddress = wallet.getCurrentAddress(); + expect(currentAddress.address).toBeDefined(); + expect(currentAddress.index).toBeDefined(); + }); + it('should reject write operations on a readonly (xpub) wallet', async () => { const walletData = adapter.getPrecalculatedWallet(); const xpub = deriveXpubFromSeed(walletData.words); diff --git a/__tests__/wallet/readOnlyWallet.test.ts b/__tests__/wallet/readOnlyWallet.test.ts index 4779f4676..a2bb6f178 100644 --- a/__tests__/wallet/readOnlyWallet.test.ts +++ b/__tests__/wallet/readOnlyWallet.test.ts @@ -135,7 +135,7 @@ describe('Read-Only Wallet Access', () => { expect(mockGetWalletStatus).not.toHaveBeenCalled(); }); - it('should poll for wallet status when wallet is creating', async () => { + it('should retry getReadOnlyAuthToken when wallet is still creating', async () => { const wallet = new HathorWalletServiceWallet({ requestPassword, xpub, @@ -146,41 +146,15 @@ describe('Read-Only Wallet Access', () => { const mockGetWalletStatus = walletApi.getWalletStatus as jest.Mock; const mockGetNewAddresses = walletApi.getNewAddresses as jest.Mock; - // First call to getReadOnlyAuthToken fails because wallet is not ready + // First two calls fail (wallet still creating), third succeeds mockCreateReadOnlyAuthToken .mockRejectedValueOnce(new WalletRequestError('Wallet not ready')) - // Second call succeeds after polling + .mockRejectedValueOnce(new WalletRequestError('Wallet not ready')) .mockResolvedValueOnce({ success: true, token: mockToken, }); - // First call returns 'creating' status - mockGetWalletStatus.mockResolvedValueOnce({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'creating', - maxGap: 20, - createdAt: 123456789, - readyAt: null, - }, - }); - - // Subsequent polling calls return 'ready' status - mockGetWalletStatus.mockResolvedValue({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'ready', - maxGap: 20, - createdAt: 123456789, - readyAt: 123456790, - }, - }); - mockGetNewAddresses.mockResolvedValueOnce({ success: true, addresses: [ @@ -192,20 +166,19 @@ describe('Read-Only Wallet Access', () => { ], }); - // Mock the connection setup // @ts-expect-error - Accessing private method for testing jest.spyOn(wallet, 'isWsEnabled').mockReturnValue(false); await wallet.startReadOnly(); expect(wallet.isReady()).toBe(true); - // getWalletStatus should be called at least twice (initial check + polling) - expect(mockGetWalletStatus).toHaveBeenCalledTimes(2); - // getReadOnlyAuthToken should be called twice (failed + succeeded after poll) - expect(mockCreateReadOnlyAuthToken).toHaveBeenCalledTimes(2); + // No getWalletStatus calls — we retry the RO token endpoint directly + expect(mockGetWalletStatus).not.toHaveBeenCalled(); + // Three attempts: 2 failures + 1 success + expect(mockCreateReadOnlyAuthToken).toHaveBeenCalledTimes(3); }); - it('should throw error if wallet is not ready', async () => { + it('should propagate non-WalletRequestError immediately without retrying', async () => { const wallet = new HathorWalletServiceWallet({ requestPassword, xpub, @@ -213,28 +186,142 @@ describe('Read-Only Wallet Access', () => { }); const mockCreateReadOnlyAuthToken = walletApi.createReadOnlyAuthToken as jest.Mock; - const mockGetWalletStatus = walletApi.getWalletStatus as jest.Mock; - // getReadOnlyAuthToken fails because wallet is not ready - mockCreateReadOnlyAuthToken.mockRejectedValue(new WalletRequestError('Wallet not ready')); + // Simulate a network error (not a WalletRequestError) + const networkError = new Error('ECONNREFUSED'); + mockCreateReadOnlyAuthToken.mockRejectedValue(networkError); - // getWalletStatus returns error state - mockGetWalletStatus.mockResolvedValue({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'error', - maxGap: 20, - createdAt: 123456789, - readyAt: null, - }, + // Should fail immediately with the original error — no 60s timeout + await expect(wallet.startReadOnly()).rejects.toThrow('ECONNREFUSED'); + // Only one attempt — did not retry + expect(mockCreateReadOnlyAuthToken).toHaveBeenCalledTimes(1); + }); + + it('should fail immediately on non-400 WalletRequestError (permanent failure)', async () => { + const wallet = new HathorWalletServiceWallet({ + requestPassword, + xpub, + network, }); - await expect(wallet.startReadOnly()).rejects.toThrow(WalletRequestError); + const mockCreateReadOnlyAuthToken = walletApi.createReadOnlyAuthToken as jest.Mock; + + // 401 is a permanent failure — should NOT be retried + const error = new WalletRequestError('Error requesting read-only auth token.', { + cause: { status: 401, data: { error: 'unauthorized' } }, + }); + mockCreateReadOnlyAuthToken.mockRejectedValue(error); + await expect(wallet.startReadOnly()).rejects.toThrow( - 'Wallet must be initialized and ready before starting in read-only mode.' + 'Error requesting read-only auth token.' ); + // Only one attempt — did not retry + expect(mockCreateReadOnlyAuthToken).toHaveBeenCalledTimes(1); + }); + + it('should retry on 400 WalletRequestError (wallet still creating)', async () => { + const wallet = new HathorWalletServiceWallet({ + requestPassword, + xpub, + network, + }); + + const mockCreateReadOnlyAuthToken = walletApi.createReadOnlyAuthToken as jest.Mock; + const mockGetNewAddresses = walletApi.getNewAddresses as jest.Mock; + + // 400 is transient (wallet still creating) — should be retried + const error400 = new WalletRequestError('Error requesting read-only auth token.', { + cause: { status: 400, data: { error: 'wallet-not-ready' } }, + }); + mockCreateReadOnlyAuthToken + .mockRejectedValueOnce(error400) + .mockResolvedValueOnce({ success: true, token: mockToken }); + + mockGetNewAddresses.mockResolvedValueOnce({ + success: true, + addresses: [ + { + address: 'WbjNdAGBWAkCS2QVpqmacKXNy8WVXatXNM', + index: 0, + addressPath: "m/44'/280'/0'/0/0", + }, + ], + }); + + // @ts-expect-error - Accessing private method for testing + jest.spyOn(wallet, 'isWsEnabled').mockReturnValue(false); + + await wallet.startReadOnly(); + + expect(wallet.isReady()).toBe(true); + // Two attempts: 1 failure (400) + 1 success + expect(mockCreateReadOnlyAuthToken).toHaveBeenCalledTimes(2); + }); + + it('should include last retry error as cause when timing out', async () => { + jest.useFakeTimers(); + try { + const wallet = new HathorWalletServiceWallet({ + requestPassword, + xpub, + network, + }); + + const mockCreateReadOnlyAuthToken = walletApi.createReadOnlyAuthToken as jest.Mock; + + mockCreateReadOnlyAuthToken.mockRejectedValue(new WalletRequestError('Wallet not ready')); + + const promise = wallet.startReadOnly(); + const caught = promise.catch((err: Error) => err); + + for (let i = 0; i < 60; i++) { + await jest.advanceTimersByTimeAsync(1000); + } + + const error = await caught; + expect(error).toBeInstanceOf(WalletRequestError); + expect(error.message).toContain('Read-only wallet startup timed out'); + // The root cause should be preserved + expect(error.cause).toBeInstanceOf(WalletRequestError); + expect((error.cause as Error).message).toBe('Wallet not ready'); + } finally { + jest.useRealTimers(); + } + }); + + it('should time out if getReadOnlyAuthToken never succeeds', async () => { + jest.useFakeTimers(); + + try { + const wallet = new HathorWalletServiceWallet({ + requestPassword, + xpub, + network, + }); + + const mockCreateReadOnlyAuthToken = walletApi.createReadOnlyAuthToken as jest.Mock; + const mockGetWalletStatus = walletApi.getWalletStatus as jest.Mock; + + // Always fail — simulates wallet stuck in error or creating state + mockCreateReadOnlyAuthToken.mockRejectedValue(new WalletRequestError('Wallet not ready')); + + const promise = wallet.startReadOnly(); + // Catch the rejection early to prevent unhandled rejection during timer advancement + const caught = promise.catch((err: Error) => err); + + // Advance through all 60 polling intervals + for (let i = 0; i < 60; i++) { + await jest.advanceTimersByTimeAsync(1000); + } + + const error = await caught; + expect(error).toBeInstanceOf(WalletRequestError); + expect(error.message).toContain('Read-only wallet startup timed out'); + // Should never call getWalletStatus (no authenticated fallback) + expect(mockGetWalletStatus).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } }); it('should throw error if xpub is not set', async () => { @@ -460,7 +547,7 @@ describe('Read-Only Wallet Access', () => { expect(mockGetNewAddresses).toHaveBeenCalledTimes(1); }); - it('should skip address fetching when skipAddressFetch is true with polling', async () => { + it('should skip address fetching when skipAddressFetch is true with retries', async () => { const wallet = new HathorWalletServiceWallet({ requestPassword, xpub, @@ -471,58 +558,30 @@ describe('Read-Only Wallet Access', () => { const mockGetWalletStatus = walletApi.getWalletStatus as jest.Mock; const mockGetNewAddresses = walletApi.getNewAddresses as jest.Mock; - // Clear any previous mock implementations mockCreateReadOnlyAuthToken.mockReset(); - mockGetWalletStatus.mockReset(); - // First call to getReadOnlyAuthToken fails because wallet is not ready + // First call fails (wallet creating), second succeeds mockCreateReadOnlyAuthToken .mockRejectedValueOnce(new WalletRequestError('Wallet not ready')) - // Second call succeeds after polling .mockResolvedValueOnce({ success: true, token: mockToken, }); - // First call returns 'creating' status, subsequent calls return 'ready' - mockGetWalletStatus - .mockResolvedValueOnce({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'creating', - maxGap: 20, - createdAt: 123456789, - readyAt: null, - }, - }) - .mockResolvedValue({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'ready', - maxGap: 20, - createdAt: 123456789, - readyAt: 123456790, - }, - }); - - // Mock the connection setup // @ts-expect-error - Accessing private method for testing jest.spyOn(wallet, 'isWsEnabled').mockReturnValue(false); await wallet.startReadOnly({ skipAddressFetch: true }); expect(wallet.isReady()).toBe(true); - // Polling should have been triggered - expect(mockGetWalletStatus).toHaveBeenCalledTimes(2); - // Even with polling, addresses should not be fetched when skipAddressFetch is true + // Retries go through getReadOnlyAuthToken, not getWalletStatus + expect(mockGetWalletStatus).not.toHaveBeenCalled(); + expect(mockCreateReadOnlyAuthToken).toHaveBeenCalledTimes(2); + // Addresses should not be fetched when skipAddressFetch is true expect(mockGetNewAddresses).not.toHaveBeenCalled(); }); - it('should fetch addresses with polling when skipAddressFetch is false', async () => { + it('should fetch addresses with retries when skipAddressFetch is false', async () => { const wallet = new HathorWalletServiceWallet({ requestPassword, xpub, @@ -530,44 +589,16 @@ describe('Read-Only Wallet Access', () => { }); const mockCreateReadOnlyAuthToken = walletApi.createReadOnlyAuthToken as jest.Mock; - const mockGetWalletStatus = walletApi.getWalletStatus as jest.Mock; const mockGetNewAddresses = walletApi.getNewAddresses as jest.Mock; - // First call to getReadOnlyAuthToken fails because wallet is not ready + // First call fails (wallet creating), second succeeds mockCreateReadOnlyAuthToken .mockRejectedValueOnce(new WalletRequestError('Wallet not ready')) - // Second call succeeds after polling .mockResolvedValueOnce({ success: true, token: mockToken, }); - // First call returns 'creating' status - mockGetWalletStatus.mockResolvedValueOnce({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'creating', - maxGap: 20, - createdAt: 123456789, - readyAt: null, - }, - }); - - // Subsequent polling calls return 'ready' status - mockGetWalletStatus.mockResolvedValue({ - success: true, - status: { - walletId: mockWalletId, - xpubkey: xpub, - status: 'ready', - maxGap: 20, - createdAt: 123456789, - readyAt: 123456790, - }, - }); - mockGetNewAddresses.mockResolvedValueOnce({ success: true, addresses: [ @@ -579,15 +610,13 @@ describe('Read-Only Wallet Access', () => { ], }); - // Mock the connection setup // @ts-expect-error - Accessing private method for testing jest.spyOn(wallet, 'isWsEnabled').mockReturnValue(false); await wallet.startReadOnly({ skipAddressFetch: false }); expect(wallet.isReady()).toBe(true); - // Addresses should be fetched even with polling - expect(mockGetNewAddresses).toHaveBeenCalled(); + // Addresses should be fetched even after retries expect(mockGetNewAddresses).toHaveBeenCalledTimes(1); }); }); diff --git a/__tests__/wallet/wallet.test.ts b/__tests__/wallet/wallet.test.ts index 9436a79fa..4d104536f 100644 --- a/__tests__/wallet/wallet.test.ts +++ b/__tests__/wallet/wallet.test.ts @@ -2839,6 +2839,11 @@ test('start', async () => { .spyOn(HathorWalletServiceWallet.prototype, 'pollForWalletStatus') .mockImplementation(() => Promise.resolve()); jest.spyOn(HathorWalletServiceWallet.prototype, 'setupConnection').mockImplementation(jest.fn()); + jest + .spyOn(HathorWalletServiceWallet.prototype, 'renewAuthToken') + .mockImplementation(async function mockRenew(this: HathorWalletServiceWallet) { + this.authToken = 'mocked-token'; + }); jest .spyOn(walletApi, 'getNewAddresses') .mockImplementation(() => Promise.resolve({ success: true, addresses: [] })); @@ -2936,6 +2941,11 @@ test('getAddressPrivKey', async () => { .spyOn(HathorWalletServiceWallet.prototype, 'pollForWalletStatus') .mockImplementation(() => Promise.resolve()); jest.spyOn(HathorWalletServiceWallet.prototype, 'setupConnection').mockImplementation(jest.fn()); + jest + .spyOn(HathorWalletServiceWallet.prototype, 'renewAuthToken') + .mockImplementation(async function mockRenew(this: HathorWalletServiceWallet) { + this.authToken = 'mocked-token'; + }); jest .spyOn(walletApi, 'getNewAddresses') .mockImplementation(() => Promise.resolve({ success: true, addresses: [] })); @@ -2988,6 +2998,11 @@ test('signMessageWithAddress', async () => { .spyOn(HathorWalletServiceWallet.prototype, 'pollForWalletStatus') .mockImplementation(() => Promise.resolve()); jest.spyOn(HathorWalletServiceWallet.prototype, 'setupConnection').mockImplementation(jest.fn()); + jest + .spyOn(HathorWalletServiceWallet.prototype, 'renewAuthToken') + .mockImplementation(async function mockRenew(this: HathorWalletServiceWallet) { + this.authToken = 'mocked-token'; + }); jest .spyOn(walletApi, 'getNewAddresses') .mockImplementation(() => Promise.resolve({ success: true, addresses: [] })); @@ -3522,6 +3537,377 @@ describe('HathorWalletServiceWallet server configuration', () => { }); }); +describe('pollForWalletStatus', () => { + const network = new Network('testnet'); + const seed = defaultWalletSeed; + let wallet: HathorWalletServiceWallet; + + beforeEach(() => { + jest.restoreAllMocks(); + wallet = new HathorWalletServiceWallet({ + requestPassword: jest.fn(), + seed, + network, + storage: new Storage(new MemoryStore()), + }); + }); + + it('should resolve immediately when status is ready on first poll', async () => { + jest.spyOn(walletApi, 'getWalletStatus').mockResolvedValue({ + success: true, + status: { + walletId: 'test-id', + xpubkey: 'test-xpub', + status: 'ready', + maxGap: 20, + createdAt: Date.now(), + readyAt: Date.now(), + }, + }); + + await expect(wallet.pollForWalletStatus()).resolves.toBeUndefined(); + expect(walletApi.getWalletStatus).toHaveBeenCalledTimes(1); + }); + + it('should poll until status becomes ready', async () => { + const creatingResponse = { + success: true, + status: { + walletId: 'test-id', + xpubkey: 'test-xpub', + status: 'creating', + maxGap: 20, + createdAt: Date.now(), + readyAt: null, + }, + }; + const readyResponse = { + success: true, + status: { ...creatingResponse.status, status: 'ready', readyAt: Date.now() }, + }; + + jest + .spyOn(walletApi, 'getWalletStatus') + .mockResolvedValueOnce(creatingResponse) + .mockResolvedValueOnce(creatingResponse) + .mockResolvedValueOnce(readyResponse); + + await expect(wallet.pollForWalletStatus()).resolves.toBeUndefined(); + expect(walletApi.getWalletStatus).toHaveBeenCalledTimes(3); + }); + + it('should reject when status is an error state', async () => { + jest.spyOn(walletApi, 'getWalletStatus').mockResolvedValue({ + success: true, + status: { + walletId: 'test-id', + xpubkey: 'test-xpub', + status: 'error', + maxGap: 20, + createdAt: Date.now(), + readyAt: null, + }, + }); + + await expect(wallet.pollForWalletStatus()).rejects.toThrow(WalletRequestError); + expect(walletApi.getWalletStatus).toHaveBeenCalledTimes(1); + }); + + it('should reject when max poll attempts are exceeded', async () => { + jest.useFakeTimers(); + + try { + jest.spyOn(walletApi, 'getWalletStatus').mockResolvedValue({ + success: true, + status: { + walletId: 'test-id', + xpubkey: 'test-xpub', + status: 'creating', + maxGap: 20, + createdAt: Date.now(), + readyAt: null, + }, + }); + + const promise = wallet.pollForWalletStatus(); + // Catch the rejection early to prevent unhandled rejection during timer advancement + const caught = promise.catch((err: Error) => err); + + // Advance through all 60 polling intervals + for (let i = 0; i < 60; i++) { + await jest.advanceTimersByTimeAsync(1000); + } + + const error = await caught; + expect(error).toBeInstanceOf(WalletRequestError); + expect(error.message).toContain('Wallet status polling timed out'); + } finally { + jest.useRealTimers(); + } + }); + + it('should retry WalletRequestError from getWalletStatus (transient)', async () => { + const creatingResponse = { + success: true, + status: { + walletId: 'test-id', + xpubkey: 'test-xpub', + status: 'creating', + maxGap: 20, + createdAt: Date.now(), + readyAt: null, + }, + }; + const readyResponse = { + success: true, + status: { ...creatingResponse.status, status: 'ready', readyAt: Date.now() }, + }; + + jest + .spyOn(walletApi, 'getWalletStatus') + .mockRejectedValueOnce(new WalletRequestError('Server error')) + .mockResolvedValueOnce(creatingResponse) + .mockResolvedValueOnce(readyResponse); + + await expect(wallet.pollForWalletStatus()).resolves.toBeUndefined(); + expect(walletApi.getWalletStatus).toHaveBeenCalledTimes(3); + }); + + it('should propagate non-WalletRequestError immediately', async () => { + jest.spyOn(walletApi, 'getWalletStatus').mockRejectedValue(new Error('Bad key')); + + await expect(wallet.pollForWalletStatus()).rejects.toThrow('Bad key'); + expect(walletApi.getWalletStatus).toHaveBeenCalledTimes(1); + }); + + it('should include last transient error as cause when timing out', async () => { + jest.useFakeTimers(); + + try { + jest + .spyOn(walletApi, 'getWalletStatus') + .mockRejectedValue(new WalletRequestError('Persistent server error')); + + const promise = wallet.pollForWalletStatus(); + const caught = promise.catch((err: Error) => err); + + for (let i = 0; i < 60; i++) { + await jest.advanceTimersByTimeAsync(1000); + } + + const error = await caught; + expect(error).toBeInstanceOf(WalletRequestError); + expect(error.message).toContain('Wallet status polling timed out'); + expect(error.cause).toBeInstanceOf(WalletRequestError); + expect((error.cause as Error).message).toBe('Persistent server error'); + } finally { + jest.useRealTimers(); + } + }); + + it('should execute polls sequentially (no stacking)', async () => { + let concurrentCalls = 0; + let maxConcurrent = 0; + let callCount = 0; + + jest.spyOn(walletApi, 'getWalletStatus').mockImplementation(async () => { + callCount++; + concurrentCalls++; + maxConcurrent = Math.max(maxConcurrent, concurrentCalls); + // Simulate a slow response + await new Promise(resolve => { + setTimeout(resolve, 10); + }); + concurrentCalls--; + + return { + success: true, + status: { + walletId: 'test-id', + xpubkey: 'test-xpub', + status: callCount >= 3 ? 'ready' : 'creating', + maxGap: 20, + createdAt: Date.now(), + readyAt: null, + }, + }; + }); + + await wallet.pollForWalletStatus(); + + expect(callCount).toBeGreaterThanOrEqual(3); + // Verify no concurrent calls happened + expect(maxConcurrent).toBe(1); + }); +}); + +describe('validateAndRenewAuthToken', () => { + const network = new Network('testnet'); + const seed = defaultWalletSeed; + let wallet: HathorWalletServiceWallet; + + beforeEach(() => { + jest.restoreAllMocks(); + wallet = new HathorWalletServiceWallet({ + requestPassword: jest.fn(), + seed, + network, + storage: new Storage(new MemoryStore()), + }); + wallet.walletId = 'test-wallet-id'; + }); + + it('should renew token proactively when usePassword is provided and token is valid', async () => { + // Set a valid (non-expired) token so we enter the else-if(usePassword) branch + const futureExp = Math.floor(Date.now() / 1000) + 3600; + const fakePayload = Buffer.from(JSON.stringify({ exp: futureExp })).toString('base64'); + wallet.authToken = `header.${fakePayload}.signature`; + + // Derive a real auth xpriv from the test seed so HDPrivateKey.fromString succeeds + const accessData = walletUtils.generateAccessDataFromSeed(seed, { + networkName: 'testnet', + password: 'pass', + pin: 'pin', + }); + const authKey = decryptData(accessData.authKey!, 'pin'); + + const renewSpy = jest + .spyOn(wallet, 'renewAuthToken') + .mockImplementation(async function mockRenew(this: HathorWalletServiceWallet) { + this.authToken = 'renewed-token'; + }); + jest.spyOn(wallet.storage, 'getAuthPrivKey').mockResolvedValue(authKey); + + await wallet.validateAndRenewAuthToken('myPassword'); + + // The proactive renewal path should be taken + expect(renewSpy).toHaveBeenCalledTimes(1); + expect(wallet.storage.getAuthPrivKey).toHaveBeenCalledWith('myPassword'); + }); + + it('should retry renewAuthToken on transient WalletRequestError', async () => { + // This guards the integration-test failure surfaced by PR CI: on a + // pre-existing ready wallet the interceptor's first auth call can hit a + // brief wallet-service settling window and fail with a transient error. + // The helper should retry within the same validateAndRenewAuthToken call + // instead of bubbling the first failure up to the caller. + wallet.authToken = null; + + const accessData = walletUtils.generateAccessDataFromSeed(seed, { + networkName: 'testnet', + password: 'pass', + pin: 'pin', + }); + const authKey = decryptData(accessData.authKey!, 'pin'); + + const renewSpy = jest + .spyOn(wallet, 'renewAuthToken') + .mockRejectedValueOnce(new WalletRequestError('transient', { cause: { status: 400 } })) + .mockImplementationOnce(async function mockRenew(this: HathorWalletServiceWallet) { + this.authToken = 'renewed-after-retry'; + }); + jest.spyOn(wallet.storage, 'getAuthPrivKey').mockResolvedValue(authKey); + + jest.useFakeTimers(); + try { + const pending = wallet.validateAndRenewAuthToken('myPassword'); + // Drain the retry interval (WALLET_STATUS_POLLING_INTERVAL = 1000ms) so + // the second attempt fires without burning a real second on the wall clock. + await jest.advanceTimersByTimeAsync(1000); + await pending; + } finally { + jest.useRealTimers(); + } + + expect(renewSpy).toHaveBeenCalledTimes(2); + expect(wallet.authToken).toBe('renewed-after-retry'); + }); +}); + +/** + * These tests lock in the contract change introduced by this PR: + * + * renewAuthToken() MUST propagate errors instead of silently swallowing them. + * + * The previous implementation wrapped the body in a try/catch that set + * `this.authToken = null` on any failure. That silent catch was the root cause + * of 403 race conditions — a background renewal could null a valid token mid-flight. + * With every caller awaiting, suppressing errors would only mask bugs. + * These tests make a future refactor that reintroduces the catch fail loudly. + */ +describe('renewAuthToken contract', () => { + const network = new Network('testnet'); + const seed = defaultWalletSeed; + let wallet: HathorWalletServiceWallet; + const fakePrivKey = { xpubkey: 'dummy-xpubkey' } as unknown as Parameters< + HathorWalletServiceWallet['renewAuthToken'] + >[0]; + + beforeEach(() => { + jest.restoreAllMocks(); + wallet = new HathorWalletServiceWallet({ + requestPassword: jest.fn(), + seed, + network, + storage: new Storage(new MemoryStore()), + }); + wallet.walletId = 'test-wallet-id'; + // signMessage requires a real HDPrivateKey — stub it out; this suite only + // cares about the behaviour surrounding the createAuthToken call. + jest.spyOn(wallet, 'signMessage').mockReturnValue('fake-signature'); + }); + + it('should set authToken when createAuthToken succeeds', async () => { + jest.spyOn(walletApi, 'createAuthToken').mockResolvedValue({ + success: true, + token: 'fresh-token', + }); + + await wallet.renewAuthToken(fakePrivKey, 1234567890); + + expect(wallet.authToken).toBe('fresh-token'); + }); + + it('should propagate WalletRequestError from createAuthToken', async () => { + const apiError = new WalletRequestError('Error requesting auth token.', { + cause: { status: 400, data: {} }, + }); + jest.spyOn(walletApi, 'createAuthToken').mockRejectedValue(apiError); + + await expect(wallet.renewAuthToken(fakePrivKey, 1234567890)).rejects.toBe(apiError); + }); + + it('should NOT null out an existing authToken when renewal throws', async () => { + // This is the critical regression guard. Prior behaviour set + // `this.authToken = null` in the catch block, turning a transient renewal + // failure into a permanent 403 for the next authenticated request. + wallet.authToken = 'pre-existing-valid-token'; + jest + .spyOn(walletApi, 'createAuthToken') + .mockRejectedValue(new WalletRequestError('boom', { cause: { status: 503 } })); + + await expect(wallet.renewAuthToken(fakePrivKey, 1234567890)).rejects.toThrow('boom'); + + expect(wallet.authToken).toBe('pre-existing-valid-token'); + }); + + it('should propagate non-WalletRequestError (e.g., programming errors)', async () => { + jest.spyOn(walletApi, 'createAuthToken').mockRejectedValue(new TypeError('unexpected')); + + await expect(wallet.renewAuthToken(fakePrivKey, 1234567890)).rejects.toBeInstanceOf(TypeError); + }); + + it('should throw when called before walletId is set', async () => { + wallet.walletId = ''; + const createSpy = jest.spyOn(walletApi, 'createAuthToken'); + + await expect(wallet.renewAuthToken(fakePrivKey, 1234567890)).rejects.toThrow( + 'Wallet not ready yet.' + ); + expect(createSpy).not.toHaveBeenCalled(); + }); +}); + describe('HathorWalletServiceWallet start method error conditions', () => { const network = new Network('testnet'); const seed = defaultWalletSeed; @@ -3574,7 +3960,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ success: true, status: { @@ -3597,7 +3987,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3635,7 +4029,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3677,7 +4075,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3720,7 +4122,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3763,7 +4169,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3793,7 +4203,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ success: true, @@ -3819,7 +4233,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3865,7 +4283,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - jest.spyOn(wallet, 'renewAuthToken').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); const mockWalletId = 'test-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ @@ -3894,15 +4316,11 @@ describe('HathorWalletServiceWallet start method error conditions', () => { expect(wallet.walletId).toBe(mockWalletId); }); - it('should handle async errors from first validateAndRenewAuthToken before await', async () => { + it('should propagate errors from onWalletReady through start()', async () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); - // Mock validateAndRenewAuthToken to reject immediately (async error before await) - const authError = new Error('Auth token validation failed'); - jest.spyOn(wallet, 'validateAndRenewAuthToken').mockRejectedValue(authError); - jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ success: true, status: { @@ -3915,48 +4333,71 @@ describe('HathorWalletServiceWallet start method error conditions', () => { }, }); - jest.spyOn(walletApi, 'getNewAddresses').mockResolvedValue({ + // Errors during onWalletReady (e.g. address fetch, auth renewal via + // the axios interceptor) should propagate through start() rather than + // being swallowed. + // @ts-expect-error - Accessing private method for testing + jest + .spyOn(wallet, 'onWalletReady') + .mockRejectedValue(new WalletRequestError('Auth invalid signature')); + + await expect(wallet.start({ pinCode: '123', password: '123' })).rejects.toThrow( + 'Auth invalid signature' + ); + }); + + it('should not explicitly call validateAndRenewAuthToken during start', async () => { + jest + .spyOn(wallet.storage, 'getAccessData') + .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); + + jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ success: true, - addresses: [ - { - address: 'test-address', - index: 0, - addressPath: "m/44'/280'/0'/0/0", - }, - ], + status: { + walletId: 'test-wallet-id', + xpubkey: 'test-xpub', + status: 'creating', + maxGap: 20, + createdAt: Date.now(), + readyAt: null, + }, }); + const validateSpy = jest + .spyOn(wallet, 'validateAndRenewAuthToken') + .mockResolvedValue(undefined); + jest.spyOn(wallet, 'pollForWalletStatus').mockResolvedValue(undefined); // @ts-expect-error - Accessing private method for testing jest.spyOn(wallet, 'onWalletReady').mockResolvedValue(undefined); - // The wallet should handle the auth error gracefully await wallet.start({ pinCode: '123', password: '123' }); - // Auth token should be cleared when error occurs - expect(wallet.authToken).toBeNull(); - - // validateAndRenewAuthToken should be called twice: - // 1. First attempt that fails - // 2. Second retry after wallet creation - expect(wallet.validateAndRenewAuthToken).toHaveBeenCalledTimes(2); + // Auth token renewal is handled by the axios interceptor (via + // axiosInstance), not by start() directly. start() should NOT call + // validateAndRenewAuthToken explicitly. + expect(validateSpy).not.toHaveBeenCalled(); }); - it('should handle async errors from second validateAndRenewAuthToken in handleCreate', async () => { + it('should set walletId exactly once from createWallet response', async () => { jest .spyOn(wallet.storage, 'getAccessData') .mockRejectedValueOnce(new UninitializedWalletError('Wallet not initialized')); + jest.spyOn(wallet, 'renewAuthToken').mockImplementation(async function mockRenew( + this: HathorWalletServiceWallet + ) { + this.authToken = 'mocked-token'; + }); - // Mock first call to succeed, but second call (retry) to fail - const authError = new Error('Auth token renewal failed on retry'); - jest - .spyOn(wallet, 'validateAndRenewAuthToken') - .mockRejectedValueOnce(new Error('First auth attempt failed')) - .mockRejectedValueOnce(authError); - + const mockWalletId = 'unique-wallet-id'; jest.spyOn(walletApi, 'createWallet').mockResolvedValue({ success: true, status: { - walletId: 'test-wallet-id', + walletId: mockWalletId, xpubkey: 'test-xpub', status: 'ready', maxGap: 20, @@ -3965,28 +4406,12 @@ describe('HathorWalletServiceWallet start method error conditions', () => { }, }); - jest.spyOn(walletApi, 'getNewAddresses').mockResolvedValue({ - success: true, - addresses: [ - { - address: 'test-address', - index: 0, - addressPath: "m/44'/280'/0'/0/0", - }, - ], - }); - // @ts-expect-error - Accessing private method for testing jest.spyOn(wallet, 'onWalletReady').mockResolvedValue(undefined); - // The wallet should handle both auth errors gracefully await wallet.start({ pinCode: '123', password: '123' }); - // Auth token should be cleared when errors occur - expect(wallet.authToken).toBeNull(); - - // validateAndRenewAuthToken should be called twice (both failed) - expect(wallet.validateAndRenewAuthToken).toHaveBeenCalledTimes(2); + expect(wallet.walletId).toBe(mockWalletId); }); }); }); diff --git a/__tests__/wallet/walletServiceRetry.test.ts b/__tests__/wallet/walletServiceRetry.test.ts new file mode 100644 index 000000000..629a584dc --- /dev/null +++ b/__tests__/wallet/walletServiceRetry.test.ts @@ -0,0 +1,110 @@ +/** + * Copyright (c) Hathor Labs and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { retryOnTransientWalletError } from '../../src/wallet/walletServiceRetry'; +import { WalletRequestError } from '../../src/errors'; + +/** + * Unit tests for the `retryOnTransientWalletError` helper. + * + * These tests focus on the retry policy in isolation — they do not touch any + * wallet state or HTTP calls. The helper is deliberately small and generic so + * it can be re-used by any code path that talks to the wallet-service and + * wants bounded tolerance for transient non-2xx responses. + */ +describe('retryOnTransientWalletError', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('should return the result on first success without retrying', async () => { + const op = jest.fn().mockResolvedValue('ok'); + + const result = await retryOnTransientWalletError(op, { maxAttempts: 3, intervalMs: 10 }); + + expect(result).toBe('ok'); + expect(op).toHaveBeenCalledTimes(1); + }); + + it('should retry on WalletRequestError and return the eventual success value', async () => { + const transient = new WalletRequestError('transient', { cause: { status: 400 } }); + const op = jest + .fn() + .mockRejectedValueOnce(transient) + .mockRejectedValueOnce(transient) + .mockResolvedValue('ok-after-retries'); + + const promise = retryOnTransientWalletError(op, { maxAttempts: 5, intervalMs: 10 }); + // Flush pending timers so the inter-attempt delays resolve. + await jest.runAllTimersAsync(); + + await expect(promise).resolves.toBe('ok-after-retries'); + expect(op).toHaveBeenCalledTimes(3); + }); + + it('should propagate non-WalletRequestError immediately without retrying', async () => { + const op = jest.fn().mockRejectedValue(new TypeError('bug')); + + await expect( + retryOnTransientWalletError(op, { maxAttempts: 5, intervalMs: 10 }) + ).rejects.toBeInstanceOf(TypeError); + expect(op).toHaveBeenCalledTimes(1); + }); + + it('should throw the last WalletRequestError once maxAttempts is exhausted', async () => { + const firstError = new WalletRequestError('first', { cause: { status: 400 } }); + const lastError = new WalletRequestError('last', { cause: { status: 400 } }); + const op = jest + .fn() + .mockRejectedValueOnce(firstError) + .mockRejectedValueOnce(firstError) + .mockRejectedValueOnce(lastError); + + const promise = retryOnTransientWalletError(op, { maxAttempts: 3, intervalMs: 10 }); + // Attach a handler eagerly so the rejection isn't flagged as unhandled + // while we advance fake timers. The assertion below is the real check. + promise.catch(() => {}); + await jest.runAllTimersAsync(); + + // The rejection carries the *most recent* transient error, so the caller + // sees the freshest diagnostic information. + await expect(promise).rejects.toBe(lastError); + expect(op).toHaveBeenCalledTimes(3); + }); + + it('should wait intervalMs between attempts (not before the first, not after the last)', async () => { + const transient = new WalletRequestError('retry me', { cause: { status: 400 } }); + const op = jest + .fn() + .mockRejectedValueOnce(transient) + .mockRejectedValueOnce(transient) + .mockResolvedValue('done'); + + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + const promise = retryOnTransientWalletError(op, { maxAttempts: 5, intervalMs: 750 }); + await jest.runAllTimersAsync(); + await promise; + + // 2 failed attempts → 2 inter-attempt delays, each for 750ms. + const delayCalls = setTimeoutSpy.mock.calls.filter(([, ms]) => ms === 750); + expect(delayCalls).toHaveLength(2); + }); + + it('should throw an Error when maxAttempts < 1 (caller misuse guard)', async () => { + const op = jest.fn(); + + await expect( + retryOnTransientWalletError(op, { maxAttempts: 0, intervalMs: 10 }) + ).rejects.toThrow(/maxAttempts/); + expect(op).not.toHaveBeenCalled(); + }); +}); diff --git a/src/wallet/api/walletApi.ts b/src/wallet/api/walletApi.ts index 3c7487017..e6165a376 100644 --- a/src/wallet/api/walletApi.ts +++ b/src/wallet/api/walletApi.ts @@ -68,7 +68,9 @@ const walletApi = { if (response.status === 200 && data.success) { return parseSchema(data, walletStatusResponseSchema); } - throw new WalletRequestError('Error getting wallet status.'); + throw new WalletRequestError('Error getting wallet status.', { + cause: { status: response.status, data: response.data }, + }); }, async getVersionData(wallet: HathorWalletServiceWallet): Promise { @@ -293,7 +295,9 @@ const walletApi = { return parseSchema(response.data, authTokenResponseSchema); } - throw new WalletRequestError('Error requesting auth token.'); + throw new WalletRequestError('Error requesting auth token.', { + cause: { status: response.status, data: response.data }, + }); }, async createReadOnlyAuthToken( @@ -309,7 +313,9 @@ const walletApi = { return parseSchema(response.data, authTokenResponseSchema); } - throw new WalletRequestError('Error requesting read-only auth token.'); + throw new WalletRequestError('Error requesting read-only auth token.', { + cause: { status: response.status, data: response.data }, + }); }, async getTxById( diff --git a/src/wallet/wallet.ts b/src/wallet/wallet.ts index 413318ecd..e0220858a 100644 --- a/src/wallet/wallet.ts +++ b/src/wallet/wallet.ts @@ -22,6 +22,7 @@ import { } from '../constants'; import { decryptData, signMessage } from '../utils/crypto'; import walletApi from './api/walletApi'; +import { retryOnTransientWalletError } from './walletServiceRetry'; import { deriveAddressFromXPubP2PKH } from '../utils/address'; import walletUtils from '../utils/wallet'; import helpers from '../utils/helpers'; @@ -44,7 +45,6 @@ import { GetBalanceObject, GetAddressesObject, GetHistoryObject, - WalletStatus, Utxo, OutputType, OutputSendTransaction, @@ -111,6 +111,22 @@ import { Fee } from '../utils/fee'; // if it ended loading and became ready const WALLET_STATUS_POLLING_INTERVAL = 1000; +// Maximum number of polling attempts before timing out +const MAX_WALLET_STATUS_POLL_ATTEMPTS = 60; + +// Wallet status values returned by the wallet-service API +const WS_STATUS_READY = 'ready'; +const WS_STATUS_CREATING = 'creating'; + +// Bounded retry budget for the wallet-service auth-token renewal. +// +// Context: right after `createWallet` returns, the `/auth/token` endpoint can +// briefly respond with a transient error while the wallet-service's internal +// state settles. +// That renewal is awaited and errors propagate, we need an explicit bounded retry +// to keep integration tests (and real clients) tolerant of the same settling race. +const MAX_AUTH_TOKEN_RENEW_ATTEMPTS = 3; + enum walletState { NOT_STARTED = 'Not started', LOADING = 'Loading', @@ -442,21 +458,6 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { await this.storage.saveAccessData(accessData); } - let renewPromise: Promise | null = null; - let renewPromiseError = null; - if (accessData.acctPathKey) { - // We can preemtively request/renew the auth token so the wallet can wait for this process - // to finish while we derive and request the wallet creation. - // This call will fail is the wallet is being created for the first time. - const acctKey = decryptData(accessData.acctPathKey, pinCode); - const privKeyAccountPath = bitcore.HDPrivateKey(acctKey); - const walletId = HathorWalletServiceWallet.getWalletIdFromXPub(privKeyAccountPath.xpubkey); - this.walletId = walletId; - renewPromise = this.validateAndRenewAuthToken(pinCode).catch(err => { - renewPromiseError = err; - }); - } - const { xpub, authXpub, @@ -468,49 +469,9 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { } = await this.generateCreateWalletAuthData(accessData, pinCode); this.firstAddress = firstAddress; - if (!renewPromise) { - // we need to start the process to renew the auth token If for any reason we had to - // derive the account path xpubkey on the method above. - const walletId = HathorWalletServiceWallet.getWalletIdFromXPub(xpub); - this.walletId = walletId; - renewPromise = this.validateAndRenewAuthToken(pinCode).catch(err => { - renewPromiseError = err; - }); - } - this.xpub = xpub; this.authPrivKey = authDerivedPrivKey; - let renewPromise2Error = null; - const handleCreate = async (data: WalletStatus, tokenRenewPromise: Promise | null) => { - this.walletId = data.walletId; - - if (data.status === 'creating') { - // If the wallet status is creating, we should wait until it is ready - // before continuing - await this.pollForWalletStatus(); - } else if (data.status !== 'ready') { - // At this stage, if the wallet is not `ready` or `creating` we should - // throw an error as there are only three states: `ready`, `creating` or `error` - throw new WalletRequestError(ErrorMessages.WALLET_STATUS_ERROR, { cause: data.status }); - } - - if (tokenRenewPromise !== null) { - try { - if (renewPromise2Error) { - // If an error happened async before this await, we must - // throw it, for it to be captured by the following catch - throw renewPromise2Error; - } - await tokenRenewPromise; - } catch (err) { - this.authToken = null; - } - } - - await this.onWalletReady(); - }; - const data = await walletApi.createWallet( this, xpub, @@ -521,7 +482,6 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { firstAddress ); - // Set walletId immediately after wallet creation this.walletId = data.status.walletId; // If waitReady is false, return immediately after wallet creation @@ -530,29 +490,25 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { return; } - // If auth token api fails we can still retry during wallet creation status polling. - let renewPromise2: Promise | null = null; - try { - if (renewPromiseError) { - // If an error happened async before this await, we must - // throw it, for it to be captured by the following catch - throw renewPromiseError; - } - // Here we await the first auth token api call before continuing the startup process. - await renewPromise; - } catch (err) { - // If the wallet was being created the api would fail, but we will try to request a new token - // now that the wallet was created and before it is ready. - this.authToken = null; - const walletId = HathorWalletServiceWallet.getWalletIdFromXPub(xpub); - this.walletId = walletId; - renewPromise2 = this.validateAndRenewAuthToken(pinCode).catch(renew2Err => { - renewPromise2Error = renew2Err; + if (data.status.status === WS_STATUS_CREATING) { + // If the wallet status is creating, we should wait until it is ready + // before continuing + await this.pollForWalletStatus(); + } else if (data.status.status !== WS_STATUS_READY) { + // At this stage, if the wallet is not 'ready' or 'creating' we should + // throw an error as there are only three states: 'ready', 'creating' or 'error' + throw new WalletRequestError(ErrorMessages.WALLET_STATUS_ERROR, { + cause: data.status, }); } - await handleCreate(data.status, renewPromise2); - + // Auth token renewal is NOT called explicitly here. The axios interceptor + // in walletServiceAxios.ts handles it on-demand: any authenticated API + // call (including getWalletStatus during polling) triggers + // validateAndRenewAuthToken() automatically when the token is missing or + // expired. By this point, authPrivKey and walletId are both set, so the + // interceptor can obtain a token without needing the PIN. + await this.onWalletReady(); this.clearSensitiveData(); } @@ -775,20 +731,37 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { * @inner */ async pollForWalletStatus(): Promise { - return new Promise((resolve, reject) => { - const pollIntervalTimer = setInterval(async () => { - const data = await walletApi.getWalletStatus(this); - - if (data.status.status === 'ready') { - clearInterval(pollIntervalTimer); - resolve(); - } else if (data.status.status !== 'creating') { - // Only possible states are 'ready', 'creating' and 'error', if status - // is not ready or creating, we should reject the promise - clearInterval(pollIntervalTimer); - reject(new WalletRequestError('Error getting wallet status.', { cause: data.status })); + let lastError: Error | null = null; + for (let attempt = 0; attempt < MAX_WALLET_STATUS_POLL_ATTEMPTS; attempt++) { + let data; + try { + data = await walletApi.getWalletStatus(this); + } catch (err) { + if (!(err instanceof WalletRequestError)) { + throw err; } - }, WALLET_STATUS_POLLING_INTERVAL); + // WalletRequestError means the server (or the auth interceptor) + // responded but said no — could be a transient 502/503 or a + // momentary auth endpoint hiccup. Retry until we exhaust attempts. + lastError = err; + await helpers.sleep(WALLET_STATUS_POLLING_INTERVAL); + continue; + } + + lastError = null; + if (data.status.status === WS_STATUS_READY) { + return; + } + // Only possible states are 'ready', 'creating' and 'error'. If status + // is not ready or creating, we should throw an error. + if (data.status.status !== WS_STATUS_CREATING) { + throw new WalletRequestError('Error getting wallet status.', { cause: data.status }); + } + + await helpers.sleep(WALLET_STATUS_POLLING_INTERVAL); + } + throw new WalletRequestError('Wallet status polling timed out.', { + cause: lastError ?? undefined, }); } @@ -1178,23 +1151,26 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { privKey = bitcore.HDPrivateKey.fromString(await this.storage.getAuthPrivKey(password)); } - await this.renewAuthToken(privKey, timestampNow); + await retryOnTransientWalletError(() => this.renewAuthToken(privKey, timestampNow), { + maxAttempts: MAX_AUTH_TOKEN_RENEW_ATTEMPTS, + intervalMs: WALLET_STATUS_POLLING_INTERVAL, + }); } else if (usePassword) { - // If we have received the user PIN, we should renew the token anyway - // without blocking this method's promise - + // Token is valid but the user provided a PIN — renew proactively. const privKey = bitcore.HDPrivateKey.fromString( await this.storage.getAuthPrivKey(usePassword) ); - this.renewAuthToken(privKey, timestampNow); + await retryOnTransientWalletError(() => this.renewAuthToken(privKey, timestampNow), { + maxAttempts: MAX_AUTH_TOKEN_RENEW_ATTEMPTS, + intervalMs: WALLET_STATUS_POLLING_INTERVAL, + }); } } /** - * Renew the auth token on the wallet service - * - * Note: This method is called in a fire-and-forget manner, so it should not throw exceptions when failing. + * Renew the auth token on the wallet service. + * Throws on failure so callers can handle the root cause. * * @param {HDPrivateKey} privKey - private key to sign the auth message * @param {number} timestamp - Current timestamp to assemble the signature @@ -1207,16 +1183,9 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { throw new Error('Wallet not ready yet.'); } - try { - const sign = this.signMessage(privKey, timestamp, this.walletId); - const data = await walletApi.createAuthToken(this, timestamp, privKey.xpubkey, sign); - - this.authToken = data.token; - } catch (err) { - // We should not throw here since this method is called in a fire-and-forget manner - // TODO: When this wallet has a logger, we should log this error to help with debugging - this.authToken = null; - } + const sign = this.signMessage(privKey, timestamp, this.walletId); + const data = await walletApi.createAuthToken(this, timestamp, privKey.xpubkey, sign); + this.authToken = data.token; } /** @@ -1273,30 +1242,37 @@ class HathorWalletServiceWallet extends EventEmitter implements IHathorWallet { const accessData = walletUtils.generateAccessDataFromXpub(this.xpub); await this.storage.saveAccessData(accessData); - // Try to get read-only auth token - // This will succeed only if wallet is in 'ready' state - try { - await this.getReadOnlyAuthToken(); - // Token obtained successfully, wallet is ready - await this.onWalletReady(skipAddressFetch); - } catch (error) { - // Token request failed, check wallet status to determine why - const walletStatus = await walletApi.getWalletStatus(this); - - if (walletStatus.status.status === 'creating') { - // Wallet is still being created, poll until it's ready - await this.pollForWalletStatus(); - // After polling completes, get the token now that wallet is ready + // Poll for read-only auth token. The RO token endpoint (no auth required) + // returns 400 while the wallet is still 'creating'. We retry WalletRequestError + // (server responded but said no) until it succeeds or we time out. + // Non-WalletRequestError (network failures, timeouts) propagate immediately. + let lastError: Error | null = null; + for (let attempt = 0; attempt < MAX_WALLET_STATUS_POLL_ATTEMPTS; attempt++) { + try { await this.getReadOnlyAuthToken(); - await this.onWalletReady(skipAddressFetch); - } else { - // Wallet is in error state or other invalid state - throw new WalletRequestError( - 'Wallet must be initialized and ready before starting in read-only mode.', - { cause: walletStatus.status } - ); + lastError = null; + break; + } catch (err) { + if (!(err instanceof WalletRequestError)) { + throw err; + } + // Only retry on HTTP 400 (wallet may still be creating). + // Other status codes (401, 403, 404, etc.) are permanent failures. + const cause = err.cause as { status?: number } | undefined; + if (cause && typeof cause.status === 'number' && cause.status !== 400) { + throw err; + } + lastError = err; + await helpers.sleep(WALLET_STATUS_POLLING_INTERVAL); } } + if (lastError) { + throw new WalletRequestError( + 'Read-only wallet startup timed out. The wallet may not be initialized or is in an error state.', + { cause: lastError } + ); + } + await this.onWalletReady(skipAddressFetch); } /** diff --git a/src/wallet/walletServiceRetry.ts b/src/wallet/walletServiceRetry.ts new file mode 100644 index 000000000..296544965 --- /dev/null +++ b/src/wallet/walletServiceRetry.ts @@ -0,0 +1,93 @@ +/** + * Copyright (c) Hathor Labs and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { WalletRequestError } from '../errors'; + +/** + * Options for {@link retryOnTransientWalletError}. + */ +export interface RetryOnTransientWalletErrorOptions { + /** + * Total number of attempts, including the initial one. Must be >= 1. + * + * Example: `maxAttempts: 3` performs one initial attempt plus up to two + * retries, for a maximum of three operation invocations. + */ + maxAttempts: number; + + /** + * Milliseconds to wait between attempts. The delay is applied *between* + * attempts only — not before the first attempt and not after the final + * (failed) attempt. + */ + intervalMs: number; +} + +/** + * Runs `operation` and retries it on transient wallet-service errors. + * + * A "transient" error is a {@link WalletRequestError}: the wallet-service + * responded with a non-2xx status code or similarly reachable-but-unhappy + * signal. These failures are expected to be short-lived — for example, the + * brief window right after wallet creation during which the `/auth/token` + * endpoint can return an error before the wallet's backend state settles. + * + * Any other rejection (network failures, programming errors, schema + * mismatches) is considered permanent and propagates immediately without + * consuming retry budget. This keeps fail-fast behaviour for real bugs while + * tolerating genuine race conditions against the wallet-service. + * + * @param operation - An async function performing a single wallet-service + * call. Must be idempotent (the helper may invoke it + * multiple times). + * @param opts - Retry policy. See {@link RetryOnTransientWalletErrorOptions}. + * + * @returns The first successful result of `operation`. + * + * @throws The last {@link WalletRequestError} if every attempt fails with a + * transient error, or the original error unchanged if a + * non-transient error is encountered. + */ +export async function retryOnTransientWalletError( + operation: () => Promise, + opts: RetryOnTransientWalletErrorOptions +): Promise { + if (opts.maxAttempts < 1) { + throw new Error( + `retryOnTransientWalletError: maxAttempts must be >= 1, got ${opts.maxAttempts}` + ); + } + + let lastTransientError: WalletRequestError | undefined; + + for (let attempt = 1; attempt <= opts.maxAttempts; attempt++) { + try { + // eslint-disable-next-line no-await-in-loop -- sequential by design + return await operation(); + } catch (err) { + if (!(err instanceof WalletRequestError)) { + // Non-transient: propagate immediately without consuming retry budget. + throw err; + } + lastTransientError = err; + const isLastAttempt = attempt === opts.maxAttempts; + if (isLastAttempt) break; + // eslint-disable-next-line no-await-in-loop -- sequential by design + await new Promise(resolve => { + setTimeout(resolve, opts.intervalMs); + }); + } + } + + // Every path that exits the loop without returning has populated + // `lastTransientError`; this guard is for TypeScript control-flow analysis + // and satisfies `no-throw-literal`. + if (!lastTransientError) { + throw new Error('retryOnTransientWalletError: invariant violated — no error captured'); + } + throw lastTransientError; +}