Skip to content
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

add a fix for conflicting Stellar transactions #5031

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { FC } from 'react';
import React, { FC, useEffect } from 'react';
import { useIntl } from 'react-intl';
import {
neutralColors,
Expand All @@ -18,6 +18,7 @@ import CopyConatainer from './CopyConatainer';
import InlineToast, { EToastType } from '@/components/toasts/InlineToast';
import { IWalletAddress } from '@/apollo/types/types';
import { Spinner } from '@/components/Spinner';
import { generateStellarPaymentQRCode } from '@/hooks/useQRCodeDonation';

interface IQRDonationCardContentProps {
tokenData?: IProjectAcceptedToken;
Expand Down Expand Up @@ -87,6 +88,23 @@ const QRDonationCardContent: FC<IQRDonationCardContentProps> = ({
}) => {
const { formatMessage } = useIntl();

const [qrData, setQRData] = React.useState<string | null>(null);

useEffect(() => {
const generateQRImage = async () => {
const qrImg = await generateStellarPaymentQRCode(
projectAddress?.address ?? '',
draftDonationData?.amount ?? 0,
projectAddress?.memo ?? '',
draftDonationData?.id,
);

setQRData(qrImg);
};

generateQRImage();
}, [projectAddress, draftDonationData]);

Comment on lines +91 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling and prevent race conditions in QR code generation.

The current implementation lacks error handling and could lead to race conditions with stale QR codes.

Apply this diff to improve the implementation:

-	const [qrData, setQRData] = React.useState<string | null>(null);
+	const [qrData, setQRData] = React.useState<{ url: string | null; error: string | null }>({
+		url: null,
+		error: null
+	});
+	const [isGeneratingQR, setIsGeneratingQR] = React.useState(false);

 	useEffect(() => {
+		let mounted = true;
 		const generateQRImage = async () => {
+			if (!projectAddress?.address || !draftDonationData?.amount) {
+				return;
+			}
+			setIsGeneratingQR(true);
 			try {
 				const qrImg = await generateStellarPaymentQRCode(
 					projectAddress?.address ?? '',
 					draftDonationData?.amount ?? 0,
 					projectAddress?.memo ?? '',
 					draftDonationData?.id,
 				);
-				setQRData(qrImg);
+				if (mounted) {
+					setQRData({ url: qrImg, error: null });
+				}
 			} catch (error) {
+				console.error('Failed to generate QR code:', error);
+				if (mounted) {
+					setQRData({ url: null, error: 'Failed to generate QR code' });
+				}
+			} finally {
+				if (mounted) {
+					setIsGeneratingQR(false);
+				}
 			}
 		};

 		generateQRImage();
+		return () => {
+			mounted = false;
+		};
 	}, [projectAddress, draftDonationData]);

Then update the ImageComponent usage:

 				<ImageComponent
-					dataUrl={qrData ?? ''}
+					dataUrl={qrData.url ?? ''}
 					isExpired={
 						!!qrDonationStatus &&
 						['expired', 'failed'].includes(qrDonationStatus)
 					}
+					error={qrData.error}
+					isLoading={isGeneratingQR}
 				/>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [qrData, setQRData] = React.useState<string | null>(null);
useEffect(() => {
const generateQRImage = async () => {
const qrImg = await generateStellarPaymentQRCode(
projectAddress?.address ?? '',
draftDonationData?.amount ?? 0,
projectAddress?.memo ?? '',
draftDonationData?.id,
);
setQRData(qrImg);
};
generateQRImage();
}, [projectAddress, draftDonationData]);
const [qrData, setQRData] = React.useState<{ url: string | null; error: string | null }>({
url: null,
error: null
});
const [isGeneratingQR, setIsGeneratingQR] = React.useState(false);
useEffect(() => {
let mounted = true;
const generateQRImage = async () => {
if (!projectAddress?.address || !draftDonationData?.amount) {
return;
}
setIsGeneratingQR(true);
try {
const qrImg = await generateStellarPaymentQRCode(
projectAddress?.address ?? '',
draftDonationData?.amount ?? 0,
projectAddress?.memo ?? '',
draftDonationData?.id,
);
if (mounted) {
setQRData({ url: qrImg, error: null });
}
} catch (error) {
console.error('Failed to generate QR code:', error);
if (mounted) {
setQRData({ url: null, error: 'Failed to generate QR code' });
}
} finally {
if (mounted) {
setIsGeneratingQR(false);
}
}
};
generateQRImage();
return () => {
mounted = false;
};
}, [projectAddress, draftDonationData]);
/* ... other component code ... */
<ImageComponent
dataUrl={qrData.url ?? ''}
isExpired={
!!qrDonationStatus &&
['expired', 'failed'].includes(qrDonationStatus)
}
error={qrData.error}
isLoading={isGeneratingQR}
/>

if (draftDonationLoading) {
return (
<Flex
Expand Down Expand Up @@ -132,7 +150,7 @@ const QRDonationCardContent: FC<IQRDonationCardContentProps> = ({
})}
</B>
<ImageComponent
dataUrl={draftDonationData?.qrCodeDataUrl ?? ''}
dataUrl={qrData ?? ''}
isExpired={
!!qrDonationStatus &&
['expired', 'failed'].includes(qrDonationStatus)
Expand Down
39 changes: 20 additions & 19 deletions src/hooks/useQRCodeDonation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,26 @@ import { IProject } from '@/apollo/types/types';

export type TQRStatus = 'waiting' | 'failed' | 'success' | 'expired';

export const generateStellarPaymentQRCode = async (
toWalletAddress: string,
amount: number,
memo = '',
draftDonationId?: number,
) => {
const formattedAddress = toWalletAddress.toUpperCase();

const paymentData = `stellar:${formattedAddress}?amount=${amount}&memo=${memo || draftDonationId}`;

try {
// Generate the QR code as a data URL
const qrCodeDataURL = await QRCode.toDataURL(paymentData);
return qrCodeDataURL;
} catch (error) {
console.error('Error generating QR code:', error);
throw error;
}
};
Comment on lines +18 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add input validation and sanitization.

The function needs validation for its parameters to prevent invalid QR codes and potential security issues.

Apply this diff to add input validation and sanitization:

 export const generateStellarPaymentQRCode = async (
 	toWalletAddress: string,
 	amount: number,
 	memo = '',
 	draftDonationId?: number,
 ) => {
+	if (!toWalletAddress?.trim()) {
+		throw new Error('Wallet address is required');
+	}
+	if (amount <= 0) {
+		throw new Error('Amount must be greater than 0');
+	}
+
 	const formattedAddress = toWalletAddress.toUpperCase();
+	const sanitizedMemo = (memo || draftDonationId || '').toString()
+		.replace(/[<>'"&]/g, ''); // Basic XSS prevention
 
-	const paymentData = `stellar:${formattedAddress}?amount=${amount}&memo=${memo || draftDonationId}`;
+	const paymentData = `stellar:${formattedAddress}?amount=${amount}&memo=${sanitizedMemo}`;
 
 	try {
 		// Generate the QR code as a data URL
 		const qrCodeDataURL = await QRCode.toDataURL(paymentData);
 		return qrCodeDataURL;
 	} catch (error) {
-		console.error('Error generating QR code:', error);
+		const errorMessage = error instanceof Error ? error.message : 'Unknown error';
+		console.error('Error generating QR code:', errorMessage);
 		throw error;
 	}
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const generateStellarPaymentQRCode = async (
toWalletAddress: string,
amount: number,
memo = '',
draftDonationId?: number,
) => {
const formattedAddress = toWalletAddress.toUpperCase();
const paymentData = `stellar:${formattedAddress}?amount=${amount}&memo=${memo || draftDonationId}`;
try {
// Generate the QR code as a data URL
const qrCodeDataURL = await QRCode.toDataURL(paymentData);
return qrCodeDataURL;
} catch (error) {
console.error('Error generating QR code:', error);
throw error;
}
};
export const generateStellarPaymentQRCode = async (
toWalletAddress: string,
amount: number,
memo = '',
draftDonationId?: number,
) => {
if (!toWalletAddress?.trim()) {
throw new Error('Wallet address is required');
}
if (amount <= 0) {
throw new Error('Amount must be greater than 0');
}
const formattedAddress = toWalletAddress.toUpperCase();
const sanitizedMemo = (memo || draftDonationId || '').toString().replace(/[<>'"&]/g, ''); // Basic XSS prevention
const paymentData = `stellar:${formattedAddress}?amount=${amount}&memo=${sanitizedMemo}`;
try {
// Generate the QR code as a data URL
const qrCodeDataURL = await QRCode.toDataURL(paymentData);
return qrCodeDataURL;
} catch (error) {
const errorMessage = error instanceof Error ? error.message : 'Unknown error';
console.error('Error generating QR code:', errorMessage);
throw error;
}
};


export const useQRCodeDonation = (project: IProject) => {
const [draftDonation, setDraftDonation] = useState<IDraftDonation | null>(
null,
Expand All @@ -23,25 +43,6 @@ export const useQRCodeDonation = (project: IProject) => {
const [loading, setLoading] = useState(false);
const [pendingDonationExists, setPendingDonationExists] = useState(false);

const generateStellarPaymentQRCode = async (
toWalletAddress: string,
amount: number,
memo = '',
) => {
const formattedAddress = toWalletAddress.toUpperCase();

const paymentData = `stellar:${formattedAddress}?amount=${amount}&memo=${memo}`;

try {
// Generate the QR code as a data URL
const qrCodeDataURL = await QRCode.toDataURL(paymentData);
return qrCodeDataURL;
} catch (error) {
console.error('Error generating QR code:', error);
throw error;
}
};

const createDraftDonation = async (
payload: ICreateDraftDonation,
): Promise<number | undefined> => {
Expand Down
Loading