Skip to content

fix: Add checkForPotentialDuplicates method for tx validation #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
140 changes: 140 additions & 0 deletions src/SmartTransactionsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,9 @@
isSmartTransactionPending,
);
if (!this.timeoutHandle && pendingTransactions?.length > 0) {
this.poll();

Check warning on line 351 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator

Check warning on line 351 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
} else if (this.timeoutHandle && pendingTransactions?.length === 0) {
this.stop();

Check warning on line 353 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator

Check warning on line 353 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}
}

Expand All @@ -375,7 +375,7 @@
}

this.timeoutHandle = setInterval(() => {
safelyExecute(async () => this.updateSmartTransactions());

Check warning on line 378 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator

Check warning on line 378 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}, this.#interval);
await safelyExecute(async () => this.updateSmartTransactions());
}
Expand Down Expand Up @@ -442,7 +442,7 @@
ethQuery = new EthQuery(provider);
}

this.#createOrUpdateSmartTransaction(smartTransaction, {

Check warning on line 445 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator

Check warning on line 445 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
chainId,
ethQuery,
});
Expand Down Expand Up @@ -729,7 +729,7 @@
: originalTxMeta;

if (this.#doesTransactionNeedConfirmation(txHash)) {
this.#confirmExternalTransaction(

Check warning on line 732 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator

Check warning on line 732 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
// TODO: Replace 'as' assertion with correct typing for `txMeta`
txMeta as TransactionMeta,
transactionReceipt,
Expand Down Expand Up @@ -836,6 +836,146 @@
};
}

/**

Check failure on line 839 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 839 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
* Extracts a readable revert reason from an error message

Check failure on line 840 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 840 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
*

Check failure on line 841 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Replace `·*·⏎` with `···*⏎··`

Check failure on line 841 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Replace `·*·⏎` with `···*⏎··`
* @param errorMessage - The full error message
* @returns The extracted revert reason or null if not found

Check failure on line 843 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 843 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
* @private

Check failure on line 844 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 844 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
*/

Check failure on line 845 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 845 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
#extractRevertReason(errorMessage: string): string | null {

Check failure on line 846 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 846 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
// Common patterns in revert messages

Check failure on line 847 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 847 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
const patterns = [

Check failure on line 848 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Insert `··`

Check failure on line 848 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Insert `··`
/execution reverted: (.*?)($|\s|\.)/i,

Check failure on line 849 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (18.x)

Replace `····` with `······`

Check failure on line 849 in src/SmartTransactionsController.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test / Lint (20.x)

Replace `····` with `······`
/reason string "(.*?)"/i,
/revert: (.*?)($|\s|\.)/i,
/revert with reason "(.*?)"/i,
/already claimed/i, // Special case for our specific scenario
];

for (const pattern of patterns) {
const match = errorMessage.match(pattern);
if (match) {
// For the "already claimed" special case or similar
if (pattern.toString().includes('already claimed')) {
return 'Already claimed';
}
return match[1];
}
}

return null;
}

/**
* Checks if there are any pending duplicate transactions that would likely fail
*
* @param params - Parameters to check for duplicates
* @param params.from - The address sending the transaction
* @param params.to - The address receiving the transaction
* @param params.data - The transaction data (function call and parameters)
* @param params.value - The transaction value in wei (optional)
* @param params.chainId - The chain ID (used indirectly through EthQuery)
* @param params.networkClientId - The network client ID (optional)
* @returns An object indicating if duplicates were found and the reason
*/
async checkForPotentialDuplicates({
from,
to,
data,
value,
chainId,
networkClientId,
}: {
from: string;
to: string;
data: string;
value?: string;
chainId: Hex;
networkClientId?: NetworkClientId;
}): Promise<{ hasDuplicates: boolean; reason?: string }> {
// log at start of function
console.log('checkForPotentialDuplicates called with', {
from: from?.substring(0, 10),
to: to?.substring(0, 10),
data: data?.substring(0, 10),
chainId
});

// Normalize addresses for comparison
const normalizedFrom = from.toLowerCase();
const normalizedTo = to.toLowerCase();

// 1. Check for pending local transactions with matching parameters
const currentSmartTransactions = this.#getCurrentSmartTransactions();

// log for current transactions
console.log('Current smart transactions count:', currentSmartTransactions.length);

const pendingDuplicates = currentSmartTransactions.filter(
(tx) =>
isSmartTransactionPending(tx) &&
tx.txParams?.from?.toLowerCase() === normalizedFrom &&
tx.txParams?.to?.toLowerCase() === normalizedTo &&
tx.txParams?.data === data
);

// log for pending duplicates
console.log('Pending duplicates found:', pendingDuplicates.length);

if (pendingDuplicates.length > 0) {
const result = {
hasDuplicates: true,
reason: 'Similar transaction is already pending'
};
console.log('checkForPotentialDuplicates result (from pending):', result);
return result;
}

// 2. Optional transaction simulation to detect potential reverts
// Only do this if we have a provider available
try {
console.log('About to get ethQuery and simulate transaction');
const ethQuery = this.#getEthQuery({ networkClientId });

// Use eth_call to simulate the transaction
console.log('Simulating transaction with eth_call');
await query(ethQuery, 'call', [{
from: normalizedFrom,
to: normalizedTo,
data,
value: value || '0x0'
}, 'latest']);

// If we get here, the simulation was successful
const result = { hasDuplicates: false };
console.log('checkForPotentialDuplicates result (no issues):', result);
return result;
} catch (error) {
// If the simulation fails, it suggests the transaction would revert
// Extract a user-friendly error message
console.log('Transaction simulation failed with error:', error);
let errorMessage = 'Transaction simulation failed';

if (error instanceof Error) {
// Try to extract a reason from the error message
const revertReason = this.#extractRevertReason(error.message);
if (revertReason) {
errorMessage = `Transaction would fail: ${revertReason}`;
} else {
errorMessage = `Transaction would fail: ${error.message}`;
}
}

const result = {
hasDuplicates: true,
reason: errorMessage
};
console.log('checkForPotentialDuplicates result (from simulation):', result);
return result;
}
}

clearFees(): Fees {
const fees = {
approvalTxFees: null,
Expand Down
Loading