WalletInfoCollector: revert more restrictive address scanning of f4b5904

f4b5904 ("Improve syncing/detection during login") changed address scanning to
strictly allow an address gap of at most ACCOUNT_MAX_ALLOWED_ADDRESS_GAP, while the
implementation we are reverting to is less restrictive by always checking full chunks
of size ACCOUNT_MAX_ALLOWED_ADDRESS_GAP, which can result in address gaps between
ACCOUNT_MAX_ALLOWED_ADDRESS_GAP and 2*ACCOUNT_MAX_ALLOWED_ADDRESS_GAP-1. This allows
to potentially find more of the user's addresses. The more exhaustive scanning is no
problem anymore now with the recent perfomance improvements of @nimiq/core.

The more detailed status messsages in LoginSuccess of f4b5904 have not been reverted
as the only remaining change. The addition of a skip button has already been reverted
previously in 1424f5f.

Author: danimoh

src/lib/WalletInfoCollector.ts

@@ -33,7 +33,6 @@ import { deriveAddressesFromXPub, getBtcNetwork, publicKeyToPayment } from './bi
 export type BasicAccountInfo = {
     address: string,
     path: string,
-    index: number,
 };
 
 export type WalletCollectionResultLedger = {
@@ -234,8 +233,8 @@ export default class WalletInfoCollector {
         }
 
         // Kick off first round of account derivation
-        // let startIndex = 0;
-        let derivedAccountsPromise = WalletInfoCollector._deriveAccounts(0 /*startIndex */,
+        let startIndex = 0;
+        let derivedAccountsPromise = WalletInfoCollector._deriveAccounts(startIndex,
             ACCOUNT_MAX_ALLOWED_ADDRESS_GAP, walletType, keyId, keyguardCookieEncryptionKey);
 
         try {
@@ -290,26 +289,25 @@ export default class WalletInfoCollector {
             }
 
             let foundAccounts: BasicAccountInfo[];
-            let lastActiveAccountIndex = 0;
             let receiptsError: Error | undefined;
             do {
                 const derivedAccounts = await derivedAccountsPromise;
 
-                // // already start deriving next accounts
-                // // By always advancing in groups of MAX_ALLOWED_GAP addresses per round, it often happens that more
-                // // addresses are derived and checked for activity than the BIP44 address gap limit
-                // // (https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#address-gap-limit) stipulates,
-                // // because whenever an active address in a group of addresses is found, the next full group is also
-                // // derived. Thus the actual gap limit of this implementation is up to (2 x MAX_ALLOWED_GAP) - 1.
-                // // We argue that this is good UX for users, as potentially more of their active addresses are found,
-                // // even if they haven't strictly followed to the standard - at only a relatively small cost to the
-                // // network. For example, if the user adds the accounts derived with indices 0, 19, 39 to his wallet but
-                // // then only ends up using accounts 0 and 39, the account at index 19 will not be found anymore on
-                // // reimport. With the current implementation however, at least the account 39 would be found, while an
-                // // implementation strictly following the specification would stop the search at index 19.
-                // startIndex += ACCOUNT_MAX_ALLOWED_ADDRESS_GAP;
-                // derivedAccountsPromise = WalletInfoCollector._deriveAccounts(startIndex,
-                //     ACCOUNT_MAX_ALLOWED_ADDRESS_GAP, walletType, keyId, keyguardCookieEncryptionKey);
+                // already start deriving next accounts
+                // By always advancing in groups of MAX_ALLOWED_GAP addresses per round, it often happens that more
+                // addresses are derived and checked for activity than the BIP44 address gap limit
+                // (https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#address-gap-limit) stipulates,
+                // because whenever an active address in a group of addresses is found, the next full group is also
+                // derived. Thus the actual gap limit of this implementation is up to (2 x MAX_ALLOWED_GAP) - 1.
+                // We argue that this is good UX for users, as potentially more of their active addresses are found,
+                // even if they haven't strictly followed to the standard - at only a relatively small cost to the
+                // network. For example, if the user adds the accounts derived with indices 0, 19, 39 to his wallet but
+                // then only ends up using accounts 0 and 39, the account at index 19 will not be found anymore on
+                // reimport. With the current implementation however, at least the account 39 would be found, while an
+                // implementation strictly following the specification would stop the search at index 19.
+                startIndex += ACCOUNT_MAX_ALLOWED_ADDRESS_GAP;
+                derivedAccountsPromise = WalletInfoCollector._deriveAccounts(startIndex,
+                    ACCOUNT_MAX_ALLOWED_ADDRESS_GAP, walletType, keyId, keyguardCookieEncryptionKey);
 
                 // Already add addresses that are in the initialAccounts
                 foundAccounts = derivedAccounts.filter((derived) =>
@@ -325,7 +323,6 @@ export default class WalletInfoCollector {
                     if (!!balance && (balance.balance !== 0 || balance.stake !== 0)) {
                         foundAccounts.push(account);
                         hasActivity = true;
-                        lastActiveAccountIndex = Math.max(lastActiveAccountIndex, account.index);
                     }
                 }
 
@@ -343,7 +340,6 @@ export default class WalletInfoCollector {
                             if (receipts.length > 0) {
                                 foundAccounts.push(account);
                                 hasActivity = true;
-                                lastActiveAccountIndex = Math.max(lastActiveAccountIndex, account.index);
                             }
                         } catch (error) {
                             if (!(error as Error).message.startsWith(ERROR_TRANSACTION_RECEIPTS)) {
@@ -359,14 +355,6 @@ export default class WalletInfoCollector {
                     WalletInfoCollector._addAccounts(walletInfo, foundAccounts, balances);
                     onUpdate(walletInfo, await derivedAccountsPromise);
                 }
-
-                // Check the remaining gap after the last checked account
-                const lastCheckedIndex = accountsToCheck[accountsToCheck.length - 1].index;
-                const nextStartIndex = lastCheckedIndex + 1;
-                const remainingGap = ACCOUNT_MAX_ALLOWED_ADDRESS_GAP - (lastCheckedIndex - lastActiveAccountIndex);
-                if (!remainingGap) break;
-                derivedAccountsPromise = WalletInfoCollector._deriveAccounts(nextStartIndex,
-                    remainingGap, walletType, keyId, keyguardCookieEncryptionKey);
             } while (foundAccounts.length > 0);
 
             const releaseKey = walletType === WalletType.BIP39
@@ -465,10 +453,8 @@ export default class WalletInfoCollector {
         keyguardCookieEncryptionKey?: Uint8Array,
     ): Promise<BasicAccountInfo[]> {
         const pathsToDerive: string[] = [];
-        const indices: number[] = [];
         for (let index = startIndex; index < startIndex + count; ++index) {
             pathsToDerive.push(`${ACCOUNT_BIP32_BASE_PATH_KEYGUARD}${index}'`);
-            indices.push(index);
         }
         const derivedAddresses = await WalletInfoCollector._keyguardClient!.deriveAddresses(keyId, pathsToDerive,
             keyguardCookieEncryptionKey);
@@ -479,29 +465,26 @@ export default class WalletInfoCollector {
             accounts.push({
                 path: pathsToDerive[i],
                 address: userFriendlyAddresses[i],
-                index: indices[i],
             });
         }
         return accounts;
     }
 
     private static async _deriveLedgerAccounts(startIndex: number, count: number): Promise<BasicAccountInfo[]> {
         const pathsToDerive: string[] = [];
-        const indices: number[] = [];
         for (let index = startIndex; index < startIndex + count; ++index) {
             pathsToDerive.push(getBip32Path({
                 coin: Coin.NIMIQ,
                 addressIndex: index,
             }));
-            indices.push(index);
         }
         return (
             await LedgerApi.Nimiq.deriveAddresses(
                 pathsToDerive,
                 /* expectedWalletId */ undefined,
                 Config.ledgerApiNimiqVersion,
             )
-        ).map((address, i) => ({ path: address.keyPath, address: address.address, index: indices[i] }));
+        ).map((address) => ({ path: address.keyPath, address: address.address }));
     }
 
     private static async _getBalances(

src/views/LoginSuccess.vue

@@ -65,10 +65,9 @@ export default class LoginSuccess extends Vue {
             await Promise.all(
                 this.keyguardResult.map(async (keyResult) => {
                     // The Keyguard always returns (at least) one derived Address,
-                    const keyguardResultAccounts = keyResult.addresses.map((addressObj, index) => ({
+                    const keyguardResultAccounts = keyResult.addresses.map((addressObj) => ({
                         address: new Nimiq.Address(addressObj.address).toUserFriendlyAddress(),
                         path: addressObj.keyPath,
-                        index,
                     }));
 
                     let tryCount = 0;
@@ -187,7 +186,6 @@ export default class LoginSuccess extends Vue {
 
     private onUpdate(walletInfo: WalletInfo, currentlyCheckedAccounts: BasicAccountInfo[]) {
         const count = !walletInfo ? 0 : walletInfo.accounts.size;
-        if (count <= 1) return;
         this.status = this.$tc('Imported {count} address so far... | Imported {count} addresses so far...', count);
     }