Restore process (#1004)
* add balance fetch, fix flush bug * adding success failed flag to restore to make sure we don't accept a failed restore * add timeout so balance call doesnt hang
This commit is contained in:
@@ -0,0 +1,147 @@
|
||||
// Regression tests for the restore "blank balance + empty history" bug.
|
||||
//
|
||||
// Root cause: getSparkTransactions swallowed every error into { transfers: [] },
|
||||
// which was indistinguishable from a genuinely empty wallet. An empty FIRST
|
||||
// batch called markRestoreComplete(), persisting isFullyRestored:true so the
|
||||
// restore poller never re-scanned. A single transient fetch failure therefore
|
||||
// left BOTH balance and history permanently empty for the session.
|
||||
//
|
||||
// The fix: getSparkTransactions now returns a `success` flag. restore only marks
|
||||
// the account complete on a SUCCESSFUL empty batch, and additionally refuses to
|
||||
// mark complete when it found zero transactions but the wallet reports a
|
||||
// positive balance (you cannot hold a balance with no transactions).
|
||||
|
||||
const mockGetSparkTransactions = jest.fn();
|
||||
const mockGetSparkBalance = jest.fn();
|
||||
|
||||
jest.mock('../../../app/functions/spark', () => ({
|
||||
getSparkTransactions: (...a) => mockGetSparkTransactions(...a),
|
||||
getSparkBalance: (...a) => mockGetSparkBalance(...a),
|
||||
sparkPaymentType: jest.fn(),
|
||||
getSingleTxDetails: jest.fn(),
|
||||
getSparkBitcoinPaymentRequest: jest.fn(),
|
||||
getSparkLightningPaymentStatus: jest.fn(),
|
||||
getSparkLightningSendRequest: jest.fn(),
|
||||
getSparkPaymentStatus: jest.fn(),
|
||||
querySparkHodlLightningPayments: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('@buildonspark/spark-sdk/types', () => ({
|
||||
LightningSendRequestStatus: {},
|
||||
SparkCoopExitRequestStatus: {},
|
||||
}));
|
||||
|
||||
jest.mock('../../../app/constants', () => ({
|
||||
IS_BITCOIN_REQUEST_ID: /^btc/,
|
||||
IS_SPARK_ID: /^spark/,
|
||||
IS_SPARK_REQUEST_ID: /^sprt/,
|
||||
}));
|
||||
|
||||
const mockSetLocalStorageItem = jest.fn();
|
||||
jest.mock('../../../app/functions/localStorage', () => ({
|
||||
getLocalStorageItem: jest.fn().mockResolvedValue(null),
|
||||
setLocalStorageItem: (...a) => mockSetLocalStorageItem(...a),
|
||||
}));
|
||||
|
||||
jest.mock('../../../app/functions/spark/transactions', () => ({
|
||||
bulkUpdateSparkTransactions: jest.fn(),
|
||||
deleteSparkTransaction: jest.fn(),
|
||||
deleteUnpaidSparkLightningTransaction: jest.fn(),
|
||||
getAllPendingSparkPayments: jest.fn().mockResolvedValue([]),
|
||||
getAllSparkTransactions: jest.fn().mockResolvedValue([]),
|
||||
getAllSparkContactInvoices: jest.fn().mockResolvedValue([]),
|
||||
getAllUnpaidSparkLightningInvoices: jest.fn().mockResolvedValue([]),
|
||||
getAllUnpaidHoldInvoicesFromTxs: jest.fn().mockResolvedValue([]),
|
||||
getBulkPaymentGroupTransferIds: jest.fn().mockResolvedValue(new Set()),
|
||||
}));
|
||||
|
||||
jest.mock('../../../app/functions/spark/transformTxToPayment', () => ({
|
||||
transformTxToPaymentObject: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('../../../app/functions/hash', () => jest.fn(() => 'hash'));
|
||||
jest.mock('../../../db/handleBackend', () => jest.fn());
|
||||
jest.mock('i18next', () => ({ t: k => k }));
|
||||
|
||||
const { fullRestoreSparkState } = require('../../../app/functions/spark/restore');
|
||||
|
||||
const ACCOUNT_ID = 'acc-1';
|
||||
const RESTORE_KEY = `spark_tx_restore_state_${ACCOUNT_ID}`;
|
||||
|
||||
// Did restore persist isFullyRestored:true for our account?
|
||||
function markedComplete() {
|
||||
return mockSetLocalStorageItem.mock.calls.some(([key, value]) => {
|
||||
if (key !== RESTORE_KEY) return false;
|
||||
try {
|
||||
return JSON.parse(value).isFullyRestored === true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
function runRestore() {
|
||||
return fullRestoreSparkState({
|
||||
sparkAddress: 'sparkAddr',
|
||||
isSendingPayment: false,
|
||||
mnemonic: 'seed words',
|
||||
identityPubKey: ACCOUNT_ID,
|
||||
sendWebViewRequest: jest.fn(),
|
||||
isInitialRestore: true,
|
||||
});
|
||||
}
|
||||
|
||||
describe('fullRestoreSparkState — restore-complete gating', () => {
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('does NOT mark restore complete when the transaction fetch fails', async () => {
|
||||
mockGetSparkTransactions.mockResolvedValue({
|
||||
transfers: [],
|
||||
success: false,
|
||||
});
|
||||
|
||||
await runRestore();
|
||||
|
||||
expect(markedComplete()).toBe(false);
|
||||
// A failed fetch must not even reach the balance sanity check.
|
||||
expect(mockGetSparkBalance).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('DOES mark restore complete on a successful empty batch for a zero-balance wallet', async () => {
|
||||
mockGetSparkTransactions.mockResolvedValue({
|
||||
transfers: [],
|
||||
success: true,
|
||||
});
|
||||
mockGetSparkBalance.mockResolvedValue({ didWork: true, balance: 0n });
|
||||
|
||||
await runRestore();
|
||||
|
||||
expect(markedComplete()).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT mark restore complete when zero txs are returned but the wallet has a balance', async () => {
|
||||
mockGetSparkTransactions.mockResolvedValue({
|
||||
transfers: [],
|
||||
success: true,
|
||||
});
|
||||
mockGetSparkBalance.mockResolvedValue({ didWork: true, balance: 5000n });
|
||||
|
||||
await runRestore();
|
||||
|
||||
expect(markedComplete()).toBe(false);
|
||||
});
|
||||
|
||||
it('marks complete on an empty batch when the balance is unknown (didWork:false), staying conservative', async () => {
|
||||
mockGetSparkTransactions.mockResolvedValue({
|
||||
transfers: [],
|
||||
success: true,
|
||||
});
|
||||
mockGetSparkBalance.mockResolvedValue({ didWork: false });
|
||||
|
||||
await runRestore();
|
||||
|
||||
expect(markedComplete()).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -1419,8 +1419,9 @@ export const getSparkTransactions = async (
|
||||
) => {
|
||||
try {
|
||||
const runtime = await selectSparkRuntime(mnemonic);
|
||||
let response;
|
||||
if (runtime === 'webview') {
|
||||
const response = await sendWebViewRequestGlobal(
|
||||
const webViewResponse = await sendWebViewRequestGlobal(
|
||||
OPERATION_TYPES.getTransactions,
|
||||
{
|
||||
mnemonic,
|
||||
@@ -1428,17 +1429,28 @@ export const getSparkTransactions = async (
|
||||
offsetIndex,
|
||||
},
|
||||
);
|
||||
return validateWebViewResponse(
|
||||
response,
|
||||
|
||||
if (webViewResponse.offset === undefined)
|
||||
throw new Error('Failed to get transfers');
|
||||
|
||||
response = validateWebViewResponse(
|
||||
webViewResponse,
|
||||
'Not able to send spark transactions',
|
||||
);
|
||||
} else {
|
||||
const wallet = await getWallet(mnemonic);
|
||||
return await wallet.getTransfers(transferCount, offsetIndex);
|
||||
response = await wallet.getTransfers(transferCount, offsetIndex);
|
||||
}
|
||||
// success:true marks a genuine reply from Spark (an empty transfers array
|
||||
// here means the wallet truly has no more transactions). Callers use this to
|
||||
// tell a real "no transactions" from a failed fetch below, which are
|
||||
// otherwise indistinguishable at the .transfers level.
|
||||
return { ...response, transfers: response?.transfers || [], success: true };
|
||||
} catch (err) {
|
||||
console.log('get spark transactions error', err);
|
||||
return { transfers: [] };
|
||||
// success:false — the fetch itself failed (network/WebView error). Callers
|
||||
// MUST NOT treat this as "no transactions" or persist a restore-complete flag.
|
||||
return { transfers: [], success: false };
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
getSparkBitcoinPaymentRequest,
|
||||
getSparkLightningPaymentStatus,
|
||||
getSparkLightningSendRequest,
|
||||
getSparkBalance,
|
||||
getSparkPaymentStatus,
|
||||
getSparkTransactions,
|
||||
querySparkHodlLightningPayments,
|
||||
@@ -33,6 +34,7 @@ import { transformTxToPaymentObject } from './transformTxToPayment';
|
||||
import sha256Hash from '../hash';
|
||||
import fetchBackend from '../../../db/handleBackend';
|
||||
import i18next from 'i18next';
|
||||
import { getBalanceWithTimeout } from '../pollingManager';
|
||||
|
||||
const RESTORE_STATE_KEY = 'spark_tx_restore_state';
|
||||
const MAX_BATCH_SIZE = 400;
|
||||
@@ -204,9 +206,34 @@ const restoreSparkTxState = async (
|
||||
while (true) {
|
||||
const txs = await getSparkTransactions(localBatchSize, offset, mnemonic);
|
||||
|
||||
if (!txs.success) {
|
||||
// The fetch failed (network/WebView error). This is indistinguishable
|
||||
// from an empty wallet at the .transfers level, so we must NOT fall
|
||||
// through to markRestoreComplete — doing so would persist
|
||||
// isFullyRestored:true and stop the restore poller from ever
|
||||
// re-scanning this session. Throw so the outer catch reports the run as
|
||||
// incomplete and the caller retries.
|
||||
throw new Error('Failed to fetch transactions during restore');
|
||||
}
|
||||
|
||||
const batchTxs = txs.transfers || [];
|
||||
|
||||
if (!batchTxs.length) {
|
||||
// A successful, empty batch normally means we've reached the end of
|
||||
// history. But if we've discovered zero transactions in total and the
|
||||
// wallet still reports a positive balance, the empty result is almost
|
||||
// certainly a bad/incomplete fetch — you cannot hold a balance with no
|
||||
// transactions. Treat that as a failure and retry rather than marking
|
||||
// the restore complete on a wallet that clearly has history.
|
||||
const noTxsDiscovered = savedIds.size === 0 && !restoredTxs.length;
|
||||
if (noTxsDiscovered) {
|
||||
const { didWork, balance } = await getBalanceWithTimeout(mnemonic);
|
||||
if (didWork && BigInt(balance || 0) > 0n) {
|
||||
throw new Error(
|
||||
'Restore returned 0 transactions but wallet has a balance; retrying',
|
||||
);
|
||||
}
|
||||
}
|
||||
console.log('No more transactions found, ending restore.');
|
||||
await markRestoreComplete(accountId);
|
||||
break;
|
||||
|
||||
@@ -215,6 +215,8 @@ const SparkWalletProvider = ({ children }) => {
|
||||
const isInitialRender = useRef(true);
|
||||
const authResetKeyRef = useRef(authResetkey);
|
||||
const balanceVersionRef = useRef(0);
|
||||
// One-shot latch: has the post-connect authoritative balance reconcile run for
|
||||
// this session yet? Reset in resetSparkState so an account switch re-arms it.
|
||||
const hasRunInitBalancePoll = useRef(false);
|
||||
const foregroundReconcileAppStateRef = useRef(appState);
|
||||
|
||||
@@ -1280,8 +1282,20 @@ const SparkWalletProvider = ({ children }) => {
|
||||
|
||||
if (!Number.isFinite(available)) return;
|
||||
// Value-gate: ignore no-op events so a burst of inbound transfers (each
|
||||
// emitting balance:update) can't trigger a render / DB-write storm.
|
||||
if (available === sparkInfoRef.current.balance) return;
|
||||
// emitting balance:update) can't trigger a render / DB-write storm. When a
|
||||
// flush is still pending, compare against the last STAGED value
|
||||
// (latestBalanceRef) rather than the committed balance. sparkInfoRef.balance
|
||||
// lags committed state by a render/effect cycle and holds the pre-burst
|
||||
// value mid-debounce, so comparing against it would drop a legitimate
|
||||
// return-to-baseline event (X→Y→X within the debounce) and leave the stale
|
||||
// intermediate Y staged to flush — an overstated balance / over-send risk.
|
||||
const hasPendingFlush =
|
||||
balanceDebounceTimeoutRef.current !== null ||
|
||||
balanceDebounceMaxWaitRef.current !== null;
|
||||
const currentTarget = hasPendingFlush
|
||||
? latestBalanceRef.current
|
||||
: sparkInfoRef.current.balance;
|
||||
if (available === currentTarget) return;
|
||||
|
||||
// Always flush with the most recent value, even when the max-wait timer
|
||||
// (set on the first event of the burst) fires.
|
||||
@@ -2257,7 +2271,7 @@ const SparkWalletProvider = ({ children }) => {
|
||||
// The init balance read timed out and painted the stale snapshot — recover
|
||||
// the real balance out-of-band so it can't stay stale until a foreground
|
||||
// cycle or manual refresh.
|
||||
if (balanceTimedOut) retryBalanceAfterTimeout();
|
||||
retryBalanceAfterTimeout();
|
||||
},
|
||||
[accountMnemoinc, sendWebViewRequest, retryBalanceAfterTimeout],
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user