From a69612996bc8dc3802b8ab6af404ee1d726cd5fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Abadesso?= Date: Fri, 6 Dec 2024 12:59:45 -0300 Subject: [PATCH] refactor: review changes --- packages/daemon/__tests__/db/index.test.ts | 14 +++--- .../__tests__/services/services.test.ts | 3 +- packages/daemon/src/db/index.ts | 36 +--------------- packages/daemon/src/services/index.ts | 43 +++++++------------ packages/daemon/src/types/db.ts | 3 +- packages/daemon/src/utils/wallet.ts | 28 ++++++++++++ 6 files changed, 57 insertions(+), 70 deletions(-) diff --git a/packages/daemon/__tests__/db/index.test.ts b/packages/daemon/__tests__/db/index.test.ts index 546fd915..930ed492 100644 --- a/packages/daemon/__tests__/db/index.test.ts +++ b/packages/daemon/__tests__/db/index.test.ts @@ -12,7 +12,6 @@ import { addUtxos, fetchAddressBalance, fetchAddressTxHistorySum, - generateAddresses, getAddressWalletInfo, getBestBlockHeight, getDbConnection, @@ -74,6 +73,7 @@ import { DbTxOutput, StringMap, TokenInfo, WalletStatus } from '../../src/types' import { Authorities, TokenBalanceMap } from '@wallet-service/common'; // @ts-ignore import { constants } from '@hathor/wallet-lib'; +import { generateAddresses } from '../../src/utils'; // Use a single mysql connection for all tests let mysql: Connection; @@ -792,7 +792,7 @@ describe('address and wallet related tests', () => { const address4 = ADDRESSES[4]; // check first with no addresses on database, so it should return only maxGap addresses - let addresses = await generateAddresses(XPUBKEY, 0, maxGap); + let addresses = await generateAddresses('mainnet', XPUBKEY, 0, maxGap); expect(Object.keys(addresses).length).toBe(maxGap); expect(addresses[address0]).toBe(0); @@ -805,14 +805,14 @@ describe('address and wallet related tests', () => { transactions: 0, }]); - addresses = await generateAddresses(XPUBKEY, 0, maxGap); + addresses = await generateAddresses('mainnet', XPUBKEY, 0, maxGap); expect(Object.keys(addresses).length).toBe(maxGap); expect(addresses[address0]).toBe(0); // now mark address0 as used let usedIndex = 0; await mysql.query('UPDATE `address` SET `transactions` = ? WHERE `address` = ?', [1, address0]); - addresses = await generateAddresses(XPUBKEY, 0, maxGap + usedIndex + 1); + addresses = await generateAddresses('mainnet', XPUBKEY, 0, maxGap + usedIndex + 1); expect(Object.keys(addresses).length).toBe(maxGap + usedIndex + 1); expect(addresses[address0]).toBe(0); @@ -825,7 +825,7 @@ describe('address and wallet related tests', () => { transactions: 1, }]); - addresses = await generateAddresses(XPUBKEY, 0, maxGap + usedIndex + 1); + addresses = await generateAddresses('mainnet', XPUBKEY, 0, maxGap + usedIndex + 1); expect(Object.keys(addresses).length).toBe(maxGap + usedIndex + 1); expect(addresses[address0]).toBe(0); expect(addresses[address1]).toBe(1); @@ -839,7 +839,7 @@ describe('address and wallet related tests', () => { transactions: 1, }]); - addresses = await generateAddresses(XPUBKEY, 0, maxGap + usedIndex + 1); + addresses = await generateAddresses('mainnet', XPUBKEY, 0, maxGap + usedIndex + 1); expect(Object.keys(addresses).length).toBe(maxGap + usedIndex + 1); expect(addresses[address0]).toBe(0); expect(addresses[address4]).toBe(4); @@ -1266,7 +1266,7 @@ describe('address generation and index methods', () => { const startIndex = 0; const count = 3; - const addresses = await generateAddresses(XPUBKEY, startIndex, count); + const addresses = await generateAddresses('mainnet', XPUBKEY, startIndex, count); // Check if we got the expected number of addresses expect(Object.keys(addresses).length).toBe(count); diff --git a/packages/daemon/__tests__/services/services.test.ts b/packages/daemon/__tests__/services/services.test.ts index 8b92b237..a57ea954 100644 --- a/packages/daemon/__tests__/services/services.test.ts +++ b/packages/daemon/__tests__/services/services.test.ts @@ -19,7 +19,6 @@ import { getUtxosLockedAtHeight, addOrUpdateTx, getAddressWalletInfo, - generateAddresses, storeTokenInformation, getMaxIndicesForWallets, } from '../../src/db'; @@ -40,6 +39,7 @@ import { getFullnodeHttpUrl, invokeOnTxPushNotificationRequestedLambda, getWalletBalancesForTx, + generateAddresses, } from '../../src/utils'; import getConfig from '../../src/config'; @@ -106,6 +106,7 @@ jest.mock('../../src/utils', () => ({ invokeOnTxPushNotificationRequestedLambda: jest.fn(), sendMessageSQS: jest.fn(), getWalletBalancesForTx: jest.fn(), + generateAddresses: jest.fn(), })); beforeEach(() => { diff --git a/packages/daemon/src/db/index.ts b/packages/daemon/src/db/index.ts index 0ae66e4d..c1da42db 100644 --- a/packages/daemon/src/db/index.ts +++ b/packages/daemon/src/db/index.ts @@ -37,8 +37,6 @@ import { TransactionRow, TxOutputRow, } from '../types'; -// @ts-ignore -import { walletUtils } from '@hathor/wallet-lib'; import getConfig from '../config'; let pool: Pool; @@ -1058,32 +1056,6 @@ export const incrementTokensTxCount = async ( `, [tokenList]); }; -/** - * Generate a batch of addresses from a given xpubkey. - * - * @remarks - * This function generates addresses starting from a specific index. - * - * @param xpubkey - The extended public key to derive addresses from - * @param startIndex - The index to start generating addresses from - * @param count - How many addresses to generate - * @returns A map of addresses to their corresponding indices - */ -export const generateAddresses = async ( - xpubkey: string, - startIndex: number, - count: number, -): Promise> => { - const { NETWORK } = getConfig(); - // We currently generate only addresses in change derivation path 0 - // (more details in https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#Change) - // so we derive our xpub to this path and use it to get the addresses - const derivedXpub = walletUtils.xpubDeriveChild(xpubkey, 0); - const addrMap = walletUtils.getAddresses(derivedXpub, startIndex, count, NETWORK); - - return addrMap; -}; - /** * Add addresses to address table. * @@ -1564,11 +1536,7 @@ export const getTokenSymbols = async ( /** * Get maximum indices for multiple wallets in a single query. - * - * @remarks - * This is an optimized version that combines both getMaxIndexAmongAddresses and getMaxWalletAddressIndex - * into a single query for multiple wallets. This reduces the number of database round trips significantly. - * + * * @param mysql - Database connection * @param walletData - Array of objects containing wallet IDs and their associated addresses * @returns Map of wallet IDs to their maximum indices (both among specific addresses and overall) @@ -1585,7 +1553,7 @@ export const getMaxIndicesForWallets = async ( const walletIds = walletData.map(d => d.walletId); const [results] = await mysql.query( - `SELECT + `SELECT wallet_id, MAX(CASE WHEN address IN (?) THEN \`index\` END) as max_among_addresses, MAX(\`index\`) as max_wallet_index diff --git a/packages/daemon/src/services/index.ts b/packages/daemon/src/services/index.ts index 1f2581d9..5df07091 100644 --- a/packages/daemon/src/services/index.ts +++ b/packages/daemon/src/services/index.ts @@ -43,6 +43,7 @@ import { getWalletBalancesForTx, getFullnodeHttpUrl, sendMessageSQS, + generateAddresses, } from '../utils'; import { getDbConnection, @@ -57,7 +58,6 @@ import { getLockedUtxoFromInputs, incrementTokensTxCount, getAddressWalletInfo, - generateAddresses, addNewAddresses, updateWalletTablesWithTx, voidTransaction, @@ -68,7 +68,7 @@ import { cleanupVoidedTx, getMaxIndicesForWallets, } from '../db'; -import getConfig, { NEW_TX_SQS } from '../config'; +import getConfig from '../config'; import logger from '../logger'; import { invokeOnTxPushNotificationRequestedLambda } from '../utils'; @@ -164,11 +164,16 @@ export const isBlock = (version: number): boolean => version === hathorLib.const export const handleVertexAccepted = async (context: Context, _event: Event) => { const mysql = await getDbConnection(); await mysql.beginTransaction(); + const { + NETWORK, + STAGE, + PUSH_NOTIFICATION_ENABLED, + NEW_TX_SQS, + } = getConfig(); try { const fullNodeEvent = context.event as FullNodeEvent; const now = getUnixTimestamp(); - const { PUSH_NOTIFICATION_ENABLED } = getConfig(); const blockRewardLock = context.rewardMinBlocks; if (!blockRewardLock) { @@ -293,13 +298,10 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { // for the addresses present on the tx, check if there are any wallets associated const addressWalletMap: StringMap = await getAddressWalletInfo(mysql, Object.keys(addressBalanceMap)); - const seenWallets = new Set(); const addressesPerWallet = Object.entries(addressWalletMap).reduce( (result: StringMap<{ addresses: string[], walletDetails: Wallet }>, [address, wallet]: [string, Wallet]) => { const { walletId } = wallet; - seenWallets.add(walletId); - // Initialize the array if the walletId is not yet a key in result if (!result[walletId]) { result[walletId] = { @@ -310,11 +312,12 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { // Add the current key to the array result[walletId].addresses.push(address); - result[walletId].walletDetails = wallet; return result; }, {}); + const seenWallets = Object.keys(addressesPerWallet); + // Convert to array format expected by getMaxIndicesForWallets const walletDataArray = Object.entries(addressesPerWallet).map(([walletId, data]) => ({ walletId, @@ -346,7 +349,7 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { if (diff < walletDetails.maxGap) { // We need to generate addresses - const addresses = await generateAddresses(walletDetails.xpubkey, maxWalletIndex + 1, walletDetails.maxGap); + const addresses = await generateAddresses(NETWORK as string, walletDetails.xpubkey, maxWalletIndex + 1, walletDetails.maxGap - diff); await addNewAddresses(mysql, walletId, addresses, maxAmongAddresses); } } @@ -355,9 +358,6 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { const walletBalanceMap: StringMap = getWalletBalanceMap(addressWalletMap, addressBalanceMap); await updateWalletTablesWithTx(mysql, hash, timestamp, walletBalanceMap); - // validate address balances - await validateAddressBalances(mysql, Object.keys(addressBalanceMap)); - // prepare the transaction data to be sent to the SQS queue const txData: Transaction = { tx_id: hash, @@ -376,7 +376,7 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { }; try { - if (seenWallets.size > 0) { + if (seenWallets.length > 0) { const queueUrl = NEW_TX_SQS; if (!queueUrl) { throw new Error('Queue URL is invalid'); @@ -406,11 +406,6 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { logger.error(e); } - const { - NETWORK, - STAGE, - } = getConfig(); - const network = new hathorLib.Network(NETWORK); // Validating for NFTs only after the tx is successfully added @@ -427,16 +422,10 @@ export const handleVertexAccepted = async (context: Context, _event: Event) => { await mysql.commit(); } catch (e) { await mysql.rollback(); - logger.error(e); - - if (e instanceof Error) { - logger.error('Error handling vertex accepted', { - error: e.message, - stack: e.stack, - }); - } else { - logger.error('Error handling vertex accepted', { error: e }); - } + logger.error('Error handling vertex accepted', { + error: (e as Error).message, + stack: (e as Error).stack, + }); throw e; } finally { diff --git a/packages/daemon/src/types/db.ts b/packages/daemon/src/types/db.ts index 2adeee20..b9685685 100644 --- a/packages/daemon/src/types/db.ts +++ b/packages/daemon/src/types/db.ts @@ -142,5 +142,6 @@ export interface TokenSymbolsRow extends RowDataPacket { } export interface MaxAddressIndexRow extends RowDataPacket { - max_index: number; + max_among_addresses: number, + max_wallet_index: number } diff --git a/packages/daemon/src/utils/wallet.ts b/packages/daemon/src/utils/wallet.ts index 4e0ac6cb..11a2b0bc 100644 --- a/packages/daemon/src/utils/wallet.ts +++ b/packages/daemon/src/utils/wallet.ts @@ -40,6 +40,8 @@ import { updateWalletLockedBalance, } from '../db'; import logger from '../logger'; +// @ts-ignore +import { walletUtils } from '@hathor/wallet-lib'; import { stringMapIterator } from './helpers'; /** @@ -507,3 +509,29 @@ export class WalletBalanceMapConverter { return walletBalanceValueMap; } } + +/** + * Generate a batch of addresses from a given xpubkey. + * + * @remarks + * This function generates addresses starting from a specific index. + * + * @param xpubkey - The extended public key to derive addresses from + * @param startIndex - The index to start generating addresses from + * @param count - How many addresses to generate + * @returns A map of addresses to their corresponding indices + */ +export const generateAddresses = async ( + network: string, + xpubkey: string, + startIndex: number, + count: number, +): Promise> => { + // We currently generate only addresses in change derivation path 0 + // (more details in https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#Change) + // so we derive our xpub to this path and use it to get the addresses + const derivedXpub = walletUtils.xpubDeriveChild(xpubkey, 0); + const addrMap = walletUtils.getAddresses(derivedXpub, startIndex, count, network); + + return addrMap; +};