From 9b9d74f99e20a77f0c2911120f2e98276f148c84 Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Wed, 19 Jun 2024 16:03:23 -0300 Subject: [PATCH 1/8] remove duplicated code on HDWallet and QiHDWallet --- src/wallet/hdwallet.ts | 111 +++++++++++++++++++++++++------------- src/wallet/qi-hdwallet.ts | 24 ++------- 2 files changed, 77 insertions(+), 58 deletions(-) diff --git a/src/wallet/hdwallet.ts b/src/wallet/hdwallet.ts index 3b272a73..4762b610 100644 --- a/src/wallet/hdwallet.ts +++ b/src/wallet/hdwallet.ts @@ -65,13 +65,13 @@ export abstract class AbstractHDWallet { this._accounts.set(accountIndex, newNode); } - protected deriveAddress( + protected deriveAddressNode( account: number, startingIndex: number, zone: Zone, isChange: boolean = false, ): HDNodeWallet { - this.validateZone(zone); + // helper method to check if derived address is valid for a given zone const isValidAddressForZone = (address: string) => { const addressZone = getZoneForAddress(address); if (!addressZone) { @@ -122,7 +122,7 @@ export abstract class AbstractHDWallet { } }); - // derive the address node + // derive the address node and validate the zone const changeIndex = isChange ? 1 : 0; const addressNode = this._root.deriveChild(account).deriveChild(changeIndex).deriveChild(addressIndex); const zone = getZoneForAddress(addressNode.address); @@ -130,19 +130,7 @@ export abstract class AbstractHDWallet { throw new Error(`Failed to derive a valid address zone for the index ${addressIndex}`); } - // create the NeuteredAddressInfo object and update the map - const neuteredAddressInfo = { - pubKey: addressNode.publicKey, - address: addressNode.address, - account: account, - index: addressNode.index, - change: isChange, - zone: zone, - }; - - addressMap.set(neuteredAddressInfo.address, neuteredAddressInfo); - - return neuteredAddressInfo; + return this.createAndStoreAddressInfo(addressNode, account, zone, isChange, addressMap); } public getNextAddress(accountIndex: number, zone: Zone): NeuteredAddressInfo { @@ -150,28 +138,10 @@ export abstract class AbstractHDWallet { if (!this._accounts.has(accountIndex)) { this.addAccount(accountIndex); } + const lastIndex = this.getLastAddressIndex(this._addresses, zone, accountIndex, false); + const addressNode = this.deriveAddressNode(accountIndex, lastIndex + 1, zone); - const filteredAccountInfos = Array.from(this._addresses.values()).filter( - (addressInfo) => addressInfo.account === accountIndex && addressInfo.zone === zone, - ); - const lastIndex = filteredAccountInfos.reduce( - (maxIndex, addressInfo) => Math.max(maxIndex, addressInfo.index), - -1, - ); - const addressNode = this.deriveAddress(accountIndex, lastIndex + 1, zone); - - // create the NeuteredAddressInfo object and update the maps - const neuteredAddressInfo = { - pubKey: addressNode.publicKey, - address: addressNode.address, - account: accountIndex, - index: addressNode.index, - change: false, - zone: zone, - }; - this._addresses.set(neuteredAddressInfo.address, neuteredAddressInfo); - - return neuteredAddressInfo; + return this.createAndStoreAddressInfo(addressNode, accountIndex, zone, false, this._addresses); } public getAddressInfo(address: string): NeuteredAddressInfo | null { @@ -371,4 +341,71 @@ export abstract class AbstractHDWallet { } } } + + /** + * Retrieves the highest address index from the given address map for a specified zone, account, and change type. + * + * This method filters the address map based on the provided zone, account, and change type, then determines the + * maximum address index from the filtered addresses. + * + * @param {Map} addressMap - The map containing address information, where the key is + * an address string and the value is a NeuteredAddressInfo object. + * @param {Zone} zone - The specific zone to filter the addresses by. + * @param {number} account - The account number to filter the addresses by. + * @param {boolean} isChange - A boolean indicating whether to filter for change addresses (true) or receiving + * addresses (false). + * + * @returns {number} - The highest address index for the specified criteria, or -1 if no addresses match. + * @protected + */ + protected getLastAddressIndex( + addressMap: Map, + zone: Zone, + account: number, + isChange: boolean, + ): number { + const addresses = Array.from(addressMap.values()).filter( + (addressInfo) => + addressInfo.account === account && addressInfo.zone === zone && addressInfo.change === isChange, + ); + return addresses.reduce((maxIndex, addressInfo) => Math.max(maxIndex, addressInfo.index), -1); + } + + /** + * Creates and stores address information in the address map for a specified account, zone, and change type. + * + * This method constructs a NeuteredAddressInfo object using the provided HDNodeWallet and other parameters, then + * stores this information in the provided address map. + * + * @param {HDNodeWallet} addressNode - The HDNodeWallet object containing the address and public key information. + * @param {number} account - The account number to associate with the address. + * @param {Zone} zone - The specific zone to associate with the address. + * @param {boolean} isChange - A boolean indicating whether the address is a change address (true) or a receiving + * address (false). + * @param {Map} addressMap - The map to store the created NeuteredAddressInfo, with the + * address as the key. + * + * @returns {NeuteredAddressInfo} - The created NeuteredAddressInfo object. + * @protected + */ + protected createAndStoreAddressInfo( + addressNode: HDNodeWallet, + account: number, + zone: Zone, + isChange: boolean, + addressMap: Map, + ): NeuteredAddressInfo { + const neuteredAddressInfo: NeuteredAddressInfo = { + pubKey: addressNode.publicKey, + address: addressNode.address, + account, + index: addressNode.index, + change: isChange, + zone, + }; + + addressMap.set(neuteredAddressInfo.address, neuteredAddressInfo); + + return neuteredAddressInfo; + } } diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index f9c17dcf..f5b016b9 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -54,28 +54,10 @@ export class QiHDWallet extends AbstractHDWallet { if (!this._accounts.has(account)) { this.addAccount(account); } - const filteredAccountInfos = Array.from(this._changeAddresses.values()).filter( - (addressInfo) => addressInfo.account === account && addressInfo.zone === zone, - ); - const lastIndex = filteredAccountInfos.reduce( - (maxIndex, addressInfo) => Math.max(maxIndex, addressInfo.index), - -1, - ); - // call derive address with change = true - const addressNode = this.deriveAddress(account, lastIndex + 1, zone, true); - - const neuteredAddressInfo = { - pubKey: addressNode.publicKey, - address: addressNode.address, - account: account, - index: addressNode.index, - change: true, - zone: zone, - }; - - this._changeAddresses.set(neuteredAddressInfo.address, neuteredAddressInfo); + const lastIndex = this.getLastAddressIndex(this._changeAddresses, zone, account, true); + const addressNode = this.deriveAddressNode(account, lastIndex + 1, zone, true); - return neuteredAddressInfo; + return this.createAndStoreAddressInfo(addressNode, account, zone, true, this._changeAddresses); } public importOutpoints(outpoints: OutpointInfo[]): void { From 7bc634fddb5f5af22a92eb901097333ebe413e0d Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Wed, 19 Jun 2024 16:04:26 -0300 Subject: [PATCH 2/8] FIX: bug in getOutpointsByAddress() --- src/providers/abstract-provider.ts | 2 +- src/wallet/qi-hdwallet.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/providers/abstract-provider.ts b/src/providers/abstract-provider.ts index f95fb142..f0a4299e 100644 --- a/src/providers/abstract-provider.ts +++ b/src/providers/abstract-provider.ts @@ -1321,7 +1321,7 @@ export class AbstractProvider implements Provider { address, 'latest', ); - return outpoints.map((outpoint: OutpointResponseParams) => ({ + return (outpoints ?? []).map((outpoint: OutpointResponseParams) => ({ txhash: outpoint.Txhash, index: outpoint.Index, denomination: outpoint.Denomination, diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index f5b016b9..417c83f4 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -280,7 +280,7 @@ export class QiHDWallet extends AbstractHDWallet { } return Object.values(outpointsMap) as Outpoint[]; } catch (error) { - throw new Error(`Failed to get outpoints for address: ${address}`); + throw new Error(`Failed to get outpoints for address: ${address} - error: ${error}`); } } From 389ff42d64961d047f9c6670c812291d703a59f2 Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Thu, 20 Jun 2024 10:23:20 -0300 Subject: [PATCH 3/8] remove _accounts map from AbstractHDWallet --- src/wallet/hdwallet.ts | 26 ++------------------------ src/wallet/qi-hdwallet.ts | 36 +++++++++++------------------------- 2 files changed, 13 insertions(+), 49 deletions(-) diff --git a/src/wallet/hdwallet.ts b/src/wallet/hdwallet.ts index 4762b610..dfa3cba3 100644 --- a/src/wallet/hdwallet.ts +++ b/src/wallet/hdwallet.ts @@ -32,9 +32,6 @@ export abstract class AbstractHDWallet { protected static _coinType?: AllowedCoinType; - // Map of account number to HDNodeWallet - protected _accounts: Map = new Map(); - // Map of addresses to address info protected _addresses: Map = new Map(); @@ -59,12 +56,6 @@ export abstract class AbstractHDWallet { return (this.constructor as typeof AbstractHDWallet)._coinType!; } - // helper methods that adds an account HD node to the HD wallet following the BIP-44 standard. - protected addAccount(accountIndex: number): void { - const newNode = this._root.deriveChild(accountIndex); - this._accounts.set(accountIndex, newNode); - } - protected deriveAddressNode( account: number, startingIndex: number, @@ -82,9 +73,8 @@ export abstract class AbstractHDWallet { return isCorrectShard && isCorrectLedger; }; // derive the address node - const accountNode = this._accounts.get(account); const changeIndex = isChange ? 1 : 0; - const changeNode = accountNode!.deriveChild(changeIndex); + const changeNode = this._root.deriveChild(account).deriveChild(changeIndex); let addrIndex: number = startingIndex; let addressNode: HDNodeWallet; do { @@ -112,9 +102,6 @@ export abstract class AbstractHDWallet { addressIndex: number, isChange: boolean = false, ): NeuteredAddressInfo { - if (!this._accounts.has(account)) { - this.addAccount(account); - } // check if address already exists for the index this._addresses.forEach((addressInfo) => { if (addressInfo.index === addressIndex) { @@ -135,9 +122,6 @@ export abstract class AbstractHDWallet { public getNextAddress(accountIndex: number, zone: Zone): NeuteredAddressInfo { this.validateZone(zone); - if (!this._accounts.has(accountIndex)) { - this.addAccount(accountIndex); - } const lastIndex = this.getLastAddressIndex(this._addresses, zone, accountIndex, false); const addressNode = this.deriveAddressNode(accountIndex, lastIndex + 1, zone); @@ -240,14 +224,8 @@ export abstract class AbstractHDWallet { throw new Error(`Address ${addr} is not known to this wallet`); } - // derive a HD node for the from address using the index - const accountNode = this._accounts.get(addressInfo.account); - if (!accountNode) { - throw new Error(`Account ${addressInfo.account} not found`); - } const changeIndex = addressInfo.change ? 1 : 0; - const changeNode = accountNode.deriveChild(changeIndex); - return changeNode.deriveChild(addressInfo.index); + return this._root.deriveChild(addressInfo.account).deriveChild(changeIndex).deriveChild(addressInfo.index); } /** diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index 417c83f4..7c7c85d6 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -51,9 +51,6 @@ export class QiHDWallet extends AbstractHDWallet { public getNextChangeAddress(account: number, zone: Zone): NeuteredAddressInfo { this.validateZone(zone); - if (!this._accounts.has(account)) { - this.addAccount(account); - } const lastIndex = this.getLastAddressIndex(this._changeAddresses, zone, account, true); const addressNode = this.deriveAddressNode(account, lastIndex + 1, zone, true); @@ -176,12 +173,11 @@ export class QiHDWallet extends AbstractHDWallet { const addressInfo = this.getAddressInfo(address); if (!addressInfo) throw new Error(`Address not found: ${address}`); // derive an HDNode for the address and get the private key - const accountNode = this._accounts.get(addressInfo.account); - if (!accountNode) { - throw new Error(`Account ${addressInfo.account} not found for address ${address}`); - } - const changeNode = accountNode.deriveChild(0); - const addressNode = changeNode.deriveChild(addressInfo.index); + const changeIndex = addressInfo.change ? 1 : 0; + const addressNode = this._root + .deriveChild(addressInfo.account) + .deriveChild(changeIndex) + .deriveChild(addressInfo.index); return addressNode.privateKey; } @@ -206,22 +202,12 @@ export class QiHDWallet extends AbstractHDWallet { // If no account is specified, it will scan all accounts known to the wallet public async sync(zone: Zone, account?: number): Promise { this.validateZone(zone); - if (account) { - await this._scan(zone, account); - } else { - for (const account of this._accounts.keys()) { - await this._scan(zone, account); - } - } + return this._scan(zone, account); } private async _scan(zone: Zone, account: number = 0): Promise { if (!this.provider) throw new Error('Provider not set'); - if (!this._accounts.has(account)) { - this.addAccount(account); - } - let gapAddressesCount = 0; let changeGapAddressesCount = 0; @@ -399,13 +385,13 @@ export class QiHDWallet extends AbstractHDWallet { outpointInfo.forEach((info) => { // validate zone this.validateZone(info.zone); - // validate address - if (!this._addresses.has(info.address)) { + // validate address and account + const addressInfo = this.getAddressInfo(info.address); + if (!addressInfo) { throw new Error(`Address ${info.address} not found in wallet`); } - // validate account - if (info.account && !this._accounts.has(info.account)) { - throw new Error(`Account ${info.account} not found in wallet`); + if (info.account !== undefined && info.account !== addressInfo.account) { + throw new Error(`Account ${info.account} not found for address ${info.address}`); } // validate Outpoint if (info.outpoint.txhash == null || info.outpoint.index == null || info.outpoint.denomination == null) { From 215c63eeb0b40db6f7d9c695cd0354c4e06a0300 Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Thu, 20 Jun 2024 10:55:02 -0300 Subject: [PATCH 4/8] rewrite 'deriveAddressNode' method --- src/wallet/hdwallet.ts | 49 ++++++++++++++++++++++++--------------- src/wallet/qi-hdwallet.ts | 2 +- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/wallet/hdwallet.ts b/src/wallet/hdwallet.ts index dfa3cba3..db900858 100644 --- a/src/wallet/hdwallet.ts +++ b/src/wallet/hdwallet.ts @@ -56,14 +56,31 @@ export abstract class AbstractHDWallet { return (this.constructor as typeof AbstractHDWallet)._coinType!; } - protected deriveAddressNode( + /** + * Derives the next valid address node for a specified account, starting index, and zone. The method ensures the + * derived address belongs to the correct shard and ledger, as defined by the Quai blockchain specifications. + * + * @param {number} account - The account number from which to derive the address node. + * @param {number} startingIndex - The index from which to start deriving addresses. + * @param {Zone} zone - The zone (shard) for which the address should be valid. + * @param {boolean} [isChange=false] - Whether to derive a change address (default is false). Default is `false` + * + * @returns {HDNodeWallet} - The derived HD node wallet containing a valid address for the specified zone. + * @throws {Error} If a valid address for the specified zone cannot be derived within the allowed attempts. + */ + protected deriveNextAddressNode( account: number, startingIndex: number, zone: Zone, isChange: boolean = false, ): HDNodeWallet { - // helper method to check if derived address is valid for a given zone - const isValidAddressForZone = (address: string) => { + const changeIndex = isChange ? 1 : 0; + const changeNode = this._root.deriveChild(account).deriveChild(changeIndex); + + let addrIndex = startingIndex; + let addressNode: HDNodeWallet; + + const isValidAddressForZone = (address: string): boolean => { const addressZone = getZoneForAddress(address); if (!addressZone) { return false; @@ -72,23 +89,17 @@ export abstract class AbstractHDWallet { const isCorrectLedger = this.coinType() === 969 ? isQiAddress(address) : !isQiAddress(address); return isCorrectShard && isCorrectLedger; }; - // derive the address node - const changeIndex = isChange ? 1 : 0; - const changeNode = this._root.deriveChild(account).deriveChild(changeIndex); - let addrIndex: number = startingIndex; - let addressNode: HDNodeWallet; - do { - addressNode = changeNode.deriveChild(addrIndex); - addrIndex++; - // put a hard limit on the number of addresses to derive - if (addrIndex - startingIndex > MAX_ADDRESS_DERIVATION_ATTEMPTS) { - throw new Error( - `Failed to derive a valid address for the zone ${zone} after ${MAX_ADDRESS_DERIVATION_ATTEMPTS} attempts.`, - ); + + for (let attempts = 0; attempts < MAX_ADDRESS_DERIVATION_ATTEMPTS; attempts++) { + addressNode = changeNode.deriveChild(addrIndex++); + if (isValidAddressForZone(addressNode.address)) { + return addressNode; } - } while (!isValidAddressForZone(addressNode.address)); + } - return addressNode; + throw new Error( + `Failed to derive a valid address for the zone ${zone} after ${MAX_ADDRESS_DERIVATION_ATTEMPTS} attempts.`, + ); } public addAddress(account: number, addressIndex: number, isChange: boolean = false): NeuteredAddressInfo { @@ -123,7 +134,7 @@ export abstract class AbstractHDWallet { public getNextAddress(accountIndex: number, zone: Zone): NeuteredAddressInfo { this.validateZone(zone); const lastIndex = this.getLastAddressIndex(this._addresses, zone, accountIndex, false); - const addressNode = this.deriveAddressNode(accountIndex, lastIndex + 1, zone); + const addressNode = this.deriveNextAddressNode(accountIndex, lastIndex + 1, zone); return this.createAndStoreAddressInfo(addressNode, accountIndex, zone, false, this._addresses); } diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index 7c7c85d6..c6b6a9f7 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -52,7 +52,7 @@ export class QiHDWallet extends AbstractHDWallet { public getNextChangeAddress(account: number, zone: Zone): NeuteredAddressInfo { this.validateZone(zone); const lastIndex = this.getLastAddressIndex(this._changeAddresses, zone, account, true); - const addressNode = this.deriveAddressNode(account, lastIndex + 1, zone, true); + const addressNode = this.deriveNextAddressNode(account, lastIndex + 1, zone, true); return this.createAndStoreAddressInfo(addressNode, account, zone, true, this._changeAddresses); } From 38c755cc9a7b386f1e1a0947cdf119e31846bfed Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Thu, 20 Jun 2024 13:38:09 -0300 Subject: [PATCH 5/8] simplify logic on 'scan' and 'nextAddress' methods --- src/wallet/hdwallet.ts | 19 ++++++++--- src/wallet/qi-hdwallet.ts | 66 +++++++++++++++------------------------ 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/src/wallet/hdwallet.ts b/src/wallet/hdwallet.ts index db900858..66185ec9 100644 --- a/src/wallet/hdwallet.ts +++ b/src/wallet/hdwallet.ts @@ -64,6 +64,7 @@ export abstract class AbstractHDWallet { * @param {number} startingIndex - The index from which to start deriving addresses. * @param {Zone} zone - The zone (shard) for which the address should be valid. * @param {boolean} [isChange=false] - Whether to derive a change address (default is false). Default is `false` + * Default is `false` * * @returns {HDNodeWallet} - The derived HD node wallet containing a valid address for the specified zone. * @throws {Error} If a valid address for the specified zone cannot be derived within the allowed attempts. @@ -131,12 +132,20 @@ export abstract class AbstractHDWallet { return this.createAndStoreAddressInfo(addressNode, account, zone, isChange, addressMap); } - public getNextAddress(accountIndex: number, zone: Zone): NeuteredAddressInfo { - this.validateZone(zone); - const lastIndex = this.getLastAddressIndex(this._addresses, zone, accountIndex, false); - const addressNode = this.deriveNextAddressNode(accountIndex, lastIndex + 1, zone); + public getNextAddress(account: number, zone: Zone): NeuteredAddressInfo { + return this._getNextAddress(account, zone, false, this._addresses); + } - return this.createAndStoreAddressInfo(addressNode, accountIndex, zone, false, this._addresses); + protected _getNextAddress( + accountIndex: number, + zone: Zone, + isChange: boolean, + addressMap: Map, + ): NeuteredAddressInfo { + this.validateZone(zone); + const lastIndex = this.getLastAddressIndex(addressMap, zone, accountIndex, isChange); + const addressNode = this.deriveNextAddressNode(accountIndex, lastIndex + 1, zone, isChange); + return this.createAndStoreAddressInfo(addressNode, accountIndex, zone, isChange, addressMap); } public getAddressInfo(address: string): NeuteredAddressInfo | null { diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index c6b6a9f7..ad3694a4 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -50,11 +50,7 @@ export class QiHDWallet extends AbstractHDWallet { } public getNextChangeAddress(account: number, zone: Zone): NeuteredAddressInfo { - this.validateZone(zone); - const lastIndex = this.getLastAddressIndex(this._changeAddresses, zone, account, true); - const addressNode = this.deriveNextAddressNode(account, lastIndex + 1, zone, true); - - return this.createAndStoreAddressInfo(addressNode, account, zone, true, this._changeAddresses); + return this._getNextAddress(account, zone, true, this._changeAddresses); } public importOutpoints(outpoints: OutpointInfo[]): void { @@ -211,52 +207,40 @@ export class QiHDWallet extends AbstractHDWallet { let gapAddressesCount = 0; let changeGapAddressesCount = 0; - // helper function to handle the common logic for both gap and change addresses - const handleAddressScanning = async ( - getAddressInfo: () => NeuteredAddressInfo, - addressesCount: number, - gapAddressesArray: NeuteredAddressInfo[], - ): Promise => { - const addressInfo = getAddressInfo(); - const outpoints = await this.getOutpointsByAddress(addressInfo.address); - if (outpoints.length === 0) { - addressesCount++; - gapAddressesArray.push(addressInfo); - } else { - addressesCount = 0; - gapAddressesArray = []; - const newOutpointsInfo = outpoints.map((outpoint) => ({ - outpoint, - address: addressInfo.address, - zone: zone, - })); - this._outpoints.push(...newOutpointsInfo); - } - return addressesCount; - }; - - // main loop to scan addresses up to the gap limit while (gapAddressesCount < QiHDWallet._GAP_LIMIT || changeGapAddressesCount < QiHDWallet._GAP_LIMIT) { [gapAddressesCount, changeGapAddressesCount] = await Promise.all([ gapAddressesCount < QiHDWallet._GAP_LIMIT - ? handleAddressScanning( - () => this.getNextAddress(account, zone), - gapAddressesCount, - this._gapAddresses, - ) + ? this.scanAddress(zone, account, false, gapAddressesCount) : gapAddressesCount, - changeGapAddressesCount < QiHDWallet._GAP_LIMIT - ? handleAddressScanning( - () => this.getNextChangeAddress(account, zone), - changeGapAddressesCount, - this._gapChangeAddresses, - ) + ? this.scanAddress(zone, account, true, changeGapAddressesCount) : changeGapAddressesCount, ]); } } + private async scanAddress(zone: Zone, account: number, isChange: boolean, addressesCount: number): Promise { + const addressMap = isChange ? this._changeAddresses : this._addresses; + const addressInfo = this._getNextAddress(account, zone, isChange, addressMap); + const outpoints = await this.getOutpointsByAddress(addressInfo.address); + if (outpoints.length > 0) { + this.importOutpoints( + outpoints.map((outpoint) => ({ + outpoint, + address: addressInfo.address, + zone, + account, + })), + ); + addressesCount = 0; + isChange ? (this._gapChangeAddresses = []) : (this._gapAddresses = []); + } else { + addressesCount++; + isChange ? this._gapChangeAddresses.push(addressInfo) : this._gapAddresses.push(addressInfo); + } + return addressesCount; + } + // getOutpointsByAddress queries the network node for the outpoints of the specified address private async getOutpointsByAddress(address: string): Promise { try { From 96eeacd1b10e810dad2ffb01d3bddbc5f5b482c9 Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Thu, 20 Jun 2024 13:50:10 -0300 Subject: [PATCH 6/8] add JSDOC comments to methods --- src/wallet/hdwallet.ts | 21 +++++++++++++- src/wallet/qi-hdwallet.ts | 58 ++++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/wallet/hdwallet.ts b/src/wallet/hdwallet.ts index 66185ec9..cd36309b 100644 --- a/src/wallet/hdwallet.ts +++ b/src/wallet/hdwallet.ts @@ -64,7 +64,7 @@ export abstract class AbstractHDWallet { * @param {number} startingIndex - The index from which to start deriving addresses. * @param {Zone} zone - The zone (shard) for which the address should be valid. * @param {boolean} [isChange=false] - Whether to derive a change address (default is false). Default is `false` - * Default is `false` + * Default is `false` Default is `false` * * @returns {HDNodeWallet} - The derived HD node wallet containing a valid address for the specified zone. * @throws {Error} If a valid address for the specified zone cannot be derived within the allowed attempts. @@ -132,10 +132,29 @@ export abstract class AbstractHDWallet { return this.createAndStoreAddressInfo(addressNode, account, zone, isChange, addressMap); } + /** + * Retrieves the next address for the specified account and zone. + * + * @param {number} account - The index of the account for which to retrieve the next address. + * @param {Zone} zone - The zone in which to retrieve the next address. + * + * @returns {NeuteredAddressInfo} The next neutered address information. + */ public getNextAddress(account: number, zone: Zone): NeuteredAddressInfo { return this._getNextAddress(account, zone, false, this._addresses); } + /** + * Derives and returns the next address information for the specified account and zone. + * + * @param {number} accountIndex - The index of the account for which the address is being generated. + * @param {Zone} zone - The zone in which the address is to be used. + * @param {boolean} isChange - A flag indicating whether the address is a change address. + * @param {Map} addressMap - A map storing the neutered address information. + * + * @returns {NeuteredAddressInfo} The derived neutered address information. + * @throws {Error} If the zone is invalid. + */ protected _getNextAddress( accountIndex: number, zone: Zone, diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index ad3694a4..fe61fa5e 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -49,6 +49,14 @@ export class QiHDWallet extends AbstractHDWallet { super(root, provider); } + /** + * Retrieves the next change address for the specified account and zone. + * + * @param {number} account - The index of the account for which to retrieve the next change address. + * @param {Zone} zone - The zone in which to retrieve the next change address. + * + * @returns {NeuteredAddressInfo} The next change neutered address information. + */ public getNextChangeAddress(account: number, zone: Zone): NeuteredAddressInfo { return this._getNextAddress(account, zone, true, this._changeAddresses); } @@ -177,9 +185,16 @@ export class QiHDWallet extends AbstractHDWallet { return addressNode.privateKey; } - // scan scans the specified zone for addresses with unspent outputs. - // Starting at index 0, tt will generate new addresses until - // the gap limit is reached for both gap and change addresses. + /** + * Scans the specified zone for addresses with unspent outputs. Starting at index 0, it will generate new addresses + * until the gap limit is reached for both gap and change addresses. + * + * @param {Zone} zone - The zone in which to scan for addresses. + * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` + * + * @returns {Promise} A promise that resolves when the scan is complete. + * @throws {Error} If the zone is invalid. + */ public async scan(zone: Zone, account: number = 0): Promise { this.validateZone(zone); // flush the existing addresses and outpoints @@ -192,15 +207,32 @@ export class QiHDWallet extends AbstractHDWallet { await this._scan(zone, account); } - // sync scans the specified zone for addresses with unspent outputs. - // Starting at the last address index, it will generate new addresses until - // the gap limit is reached for both gap and change addresses. - // If no account is specified, it will scan all accounts known to the wallet + /** + * Scans the specified zone for addresses with unspent outputs. Starting at the last address index, it will generate + * new addresses until the gap limit is reached for both gap and change addresses. If no account is specified, it + * will scan all accounts known to the wallet. + * + * @param {Zone} zone - The zone in which to sync addresses. + * @param {number} [account] - The index of the account to sync. If not specified, all accounts will be scanned. + * + * @returns {Promise} A promise that resolves when the sync is complete. + * @throws {Error} If the zone is invalid. + */ public async sync(zone: Zone, account?: number): Promise { this.validateZone(zone); return this._scan(zone, account); } + /** + * Internal method to scan the specified zone for addresses with unspent outputs. This method handles the actual + * scanning logic, generating new addresses until the gap limit is reached for both gap and change addresses. + * + * @param {Zone} zone - The zone in which to scan for addresses. + * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` + * + * @returns {Promise} A promise that resolves when the scan is complete. + * @throws {Error} If the provider is not set. + */ private async _scan(zone: Zone, account: number = 0): Promise { if (!this.provider) throw new Error('Provider not set'); @@ -219,6 +251,18 @@ export class QiHDWallet extends AbstractHDWallet { } } + /** + * Scans for the next address in the specified zone and account, checking for associated outpoints, and updates the + * address count and gap addresses accordingly. + * + * @param {Zone} zone - The zone in which the address is being scanned. + * @param {number} account - The index of the account for which the address is being scanned. + * @param {boolean} isChange - A flag indicating whether the address is a change address. + * @param {number} addressesCount - The current count of addresses scanned. + * + * @returns {Promise} A promise that resolves to the updated address count. + * @throws {Error} If an error occurs during the address scanning or outpoints retrieval process. + */ private async scanAddress(zone: Zone, account: number, isChange: boolean, addressesCount: number): Promise { const addressMap = isChange ? this._changeAddresses : this._addresses; const addressInfo = this._getNextAddress(account, zone, isChange, addressMap); From c8771d71d13324787e5cd31cb7bc7d7cae616ef2 Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Thu, 20 Jun 2024 14:30:02 -0300 Subject: [PATCH 7/8] extend sync method to sync all accounts --- src/wallet/qi-hdwallet.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index fe61fa5e..be36102b 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -190,7 +190,7 @@ export class QiHDWallet extends AbstractHDWallet { * until the gap limit is reached for both gap and change addresses. * * @param {Zone} zone - The zone in which to scan for addresses. - * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` + * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` Default is `0` * * @returns {Promise} A promise that resolves when the scan is complete. * @throws {Error} If the zone is invalid. @@ -220,7 +220,23 @@ export class QiHDWallet extends AbstractHDWallet { */ public async sync(zone: Zone, account?: number): Promise { this.validateZone(zone); - return this._scan(zone, account); + // if no account is specified, scan all accounts. + if (account === undefined) { + const addressInfos = Array.from(this._addresses.values()); + const accounts = addressInfos.reduce((unique, info) => { + if (!unique.includes(info.account)) { + unique.push(info.account); + } + return unique; + }, []); + + for (const acc of accounts) { + await this._scan(zone, acc); + } + } else { + await this._scan(zone, account); + } + return; } /** @@ -228,7 +244,7 @@ export class QiHDWallet extends AbstractHDWallet { * scanning logic, generating new addresses until the gap limit is reached for both gap and change addresses. * * @param {Zone} zone - The zone in which to scan for addresses. - * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` + * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` Default is `0` * * @returns {Promise} A promise that resolves when the scan is complete. * @throws {Error} If the provider is not set. From bb362f4684121a606e1298ab6cedf788cc4b5e45 Mon Sep 17 00:00:00 2001 From: Alejo Acosta Date: Thu, 20 Jun 2024 14:33:46 -0300 Subject: [PATCH 8/8] rename method for clarity --- src/wallet/qi-hdwallet.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/wallet/qi-hdwallet.ts b/src/wallet/qi-hdwallet.ts index be36102b..bd03d069 100644 --- a/src/wallet/qi-hdwallet.ts +++ b/src/wallet/qi-hdwallet.ts @@ -124,7 +124,7 @@ export class QiHDWallet extends AbstractHDWallet { // createSchnorrSignature returns a schnorr signature for the given message and private key private createSchnorrSignature(input: TxInput, hash: Uint8Array): string { - const privKey = this.derivePrivateKeyForInput(input); + const privKey = this.getPrivateKeyForTxInput(input); const signature = schnorr.sign(hash, getBytes(privKey)); return hexlify(signature); } @@ -137,7 +137,7 @@ export class QiHDWallet extends AbstractHDWallet { // Collect private keys corresponding to the pubkeys found on the inputs const privKeysSet = new Set(); tx.txInputs!.forEach((input) => { - const privKey = this.derivePrivateKeyForInput(input); + const privKey = this.getPrivateKeyForTxInput(input); privKeysSet.add(privKey); }); const privKeys = Array.from(privKeysSet); @@ -169,8 +169,24 @@ export class QiHDWallet extends AbstractHDWallet { return hexlify(finalSignature); } - // Helper method that returns the private key for the public key - private derivePrivateKeyForInput(input: TxInput): string { + /** + * Retrieves the private key for a given transaction input. + * + * This method derives the private key for a transaction input by following these steps: + * + * 1. Ensures the input contains a public key. + * 2. Computes the address from the public key. + * 3. Fetches address information associated with the computed address. + * 4. Derives the hierarchical deterministic (HD) node corresponding to the address. + * 5. Returns the private key of the derived HD node. + * + * @private + * @param {TxInput} input - The transaction input containing the public key. + * + * @returns {string} The private key corresponding to the transaction input. + * @throws {Error} If the input does not contain a public key or if the address information cannot be found. + */ + private getPrivateKeyForTxInput(input: TxInput): string { if (!input.pubkey) throw new Error('Missing public key for input'); const address = computeAddress(input.pubkey); // get address info @@ -191,6 +207,7 @@ export class QiHDWallet extends AbstractHDWallet { * * @param {Zone} zone - The zone in which to scan for addresses. * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` Default is `0` + * Default is `0` * * @returns {Promise} A promise that resolves when the scan is complete. * @throws {Error} If the zone is invalid. @@ -245,6 +262,7 @@ export class QiHDWallet extends AbstractHDWallet { * * @param {Zone} zone - The zone in which to scan for addresses. * @param {number} [account=0] - The index of the account to scan. Defaults to 0. Default is `0` Default is `0` + * Default is `0` * * @returns {Promise} A promise that resolves when the scan is complete. * @throws {Error} If the provider is not set.