Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 79 additions & 19 deletions src/reducers/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,29 +507,80 @@ export const getTransactions = (): AppThunk => async (dispatch, getState) => {

// Process convert transactions using the extracted utility function
if (convertedTransactions && convertedTransactions.length > 0) {
const {processedTransactions, processedTxHashes} =
await processConvertTransactions(
convertedTransactions,
lndTransactions,
(timestamp: number) => getPriceOnDate(timestamp, torEnabled),
);
// Pre-filter to only process uncached converted transactions
const uncachedConvertedTransactions: IConvertedTx[] = [];
const cachedConvertTransactions: {
convertTx: IConvertedTx;
cachedTx: IDecodedTx;
mainTx: any;
}[] = [];
Comment on lines +512 to +516
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The mainTx property is typed as any, which undermines TypeScript's type safety. You can provide a more specific type by inferring it from lndTransactions.transactions. This improves code clarity and allows for better static analysis.

Suggested change
const cachedConvertTransactions: {
convertTx: IConvertedTx;
cachedTx: IDecodedTx;
mainTx: any;
}[] = [];
const cachedConvertTransactions: {
convertTx: IConvertedTx;
cachedTx: IDecodedTx;
mainTx: (typeof lndTransactions.transactions)[0];
}[] = [];


for (const convertTx of convertedTransactions) {
// Find the main transaction for this convert operation
const mainTx = lndTransactions.transactions.find(tx => {
const hasDestinationAddress = tx.outputDetails?.some(
output => output.address === convertTx.destinationAddress,
);

if (!hasDestinationAddress) {
return false;
}

// Additional validation: check if transaction uses selected outpoints
const selectedOutpointSet = new Set(
convertTx.selectedOutpoints || [],
);
Comment on lines +530 to +532
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The selectedOutpointSet is being re-created on every iteration of the lndTransactions.transactions.find() callback. Since this set is constant for a given convertTx, it's inefficient to rebuild it repeatedly. For better performance, you should create this Set once, before the .find() method is called.

const usesSelectedOutpoints = tx.previousOutpoints?.some(
prevOutpoint =>
selectedOutpointSet.has(prevOutpoint.outpoint || ''),
);

// Additional validation: check amount proximity to target amount
const amountMatches =
Math.abs(Number(tx.amount) - Number(convertTx.targetAmount)) <
Math.max(1000, Number(convertTx.targetAmount) * 0.01);

// Add processed convert transactions to the txs array
processedTransactions.forEach(processedTx => {
// NOTE: skip processing if tx was cached before
// TODO: move the skipping to the processConvertTransactions
// when it's finished and stable to avoid getPriceOnDate calls
// Additional validation: check timestamp proximity (within 1 hour)
const timestampMatches =
Math.abs(Number(tx.timeStamp) - convertTx.timestamp) < 3600;
Comment on lines +539 to +545
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This code uses several magic numbers (1000, 0.01, 3600) for proximity checks. This makes the logic less clear. It's a good practice to extract these values into named constants (e.g., AMOUNT_TOLERANCE_SATS, AMOUNT_TOLERANCE_PERCENTAGE, TIMESTAMP_TOLERANCE_SECONDS) to improve readability and maintainability.


return usesSelectedOutpoints || (amountMatches && timestampMatches);
});
Comment on lines +520 to +548
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The logic to find the mainTx is nearly identical to the logic within processConvertTransactions. This code duplication can lead to maintenance issues, as changes would need to be made in two places. It would be best to extract this matching logic into a shared utility function that can be called from both getTransactions and processConvertTransactions.


if (!mainTx?.txHash) {
// Can't find main transaction, include for processing
uncachedConvertedTransactions.push(convertTx);
continue;
}

// Check if already cached
const cachedTx = checkTxCache(
cachedTxHashSet,
transactionsByHash,
processedTx.txHash,
mainTx.txHash,
);

if (cachedTx) {
txs.push({
...cachedTx,
numConfirmations: processedTx.numConfirmations,
});
// Transaction is cached, store for later processing
cachedConvertTransactions.push({convertTx, cachedTx, mainTx});
processedConvertTxHashes.add(mainTx.txHash);
} else {
// Not cached, needs processing
uncachedConvertedTransactions.push(convertTx);
}
}

// Process only uncached converted transactions
if (uncachedConvertedTransactions.length > 0) {
const {processedTransactions, processedTxHashes} =
await processConvertTransactions(
uncachedConvertedTransactions,
lndTransactions,
(timestamp: number) => getPriceOnDate(timestamp, torEnabled),
);

// Add newly processed convert transactions to the txs array
processedTransactions.forEach(processedTx => {
const decodedTx: IDecodedTx = {
txHash: processedTx.txHash,
blockHash: processedTx.blockHash,
Expand All @@ -547,10 +598,19 @@ export const getTransactions = (): AppThunk => async (dispatch, getState) => {
};
txs.push(decodedTx);
cachedTxHashesBuf.push(decodedTx.txHash);
}
});
});

// Merge the processed transaction hashes
processedTxHashes.forEach(hash => processedConvertTxHashes.add(hash));
}

processedConvertTxHashes = processedTxHashes;
// Add cached convert transactions to the txs array
cachedConvertTransactions.forEach(({cachedTx, mainTx}) => {
txs.push({
...cachedTx,
numConfirmations: mainTx.numConfirmations,
});
});
}

// Compare nexus-api txs with lnd txs to append missing ones in lnd
Expand Down