-
Notifications
You must be signed in to change notification settings - Fork 19
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 #1943
base: staging
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Transaction
participant F as checkTransactions
T->>F: Send transaction (type, memo, toWalletMemo, id)
F->>F: Check if type is 'payment'
alt toWalletMemo is defined
F->>F: Compare memo vs. toWalletMemo
alt Mismatch
F->>F: Log debug message and return
else
F->>F: Continue processing transaction
end
else
F->>F: Compare memo vs. id (as string)
alt Mismatch
F->>F: Log alternate debug message and return
else
F->>F: Continue processing transaction
end
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/cronJobs/checkQRTransactionJob.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
} else if (memo !== id.toString()) { | ||
logger.debug( | ||
`Transaction memo matches donation memo for donation ID ${donation.id}`, | ||
); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add type validation before toString comparison.
The memo comparison with id.toString()
assumes the memo is a string or can be safely compared. Add type validation to ensure correct comparison.
- } else if (memo !== id.toString()) {
+ } else if (typeof memo !== 'string' || memo !== id.toString()) {
📝 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.
} else if (memo !== id.toString()) { | |
logger.debug( | |
`Transaction memo matches donation memo for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} else if (typeof memo !== 'string' || memo !== id.toString()) { | |
logger.debug( | |
`Transaction memo matches donation memo for donation ID ${donation.id}`, | |
); | |
return; | |
} |
logger.debug( | ||
`Transaction memo matches donation memo for donation ID ${donation.id}`, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect debug message.
The debug message indicates a match when the condition actually checks for a mismatch.
- `Transaction memo matches donation memo for donation ID ${donation.id}`,
+ `Transaction memo does not match donation ID for donation ID ${donation.id}`,
📝 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.
logger.debug( | |
`Transaction memo matches donation memo for donation ID ${donation.id}`, | |
); | |
logger.debug( | |
`Transaction memo does not match donation ID for donation ID ${donation.id}`, | |
); |
if (transaction.type === 'payment') { | ||
if (toWalletMemo) { | ||
if (memo !== toWalletMemo) { | ||
logger.debug( | ||
`Transaction memo does not match donation memo for donation ID ${donation.id}`, | ||
); | ||
return; | ||
} | ||
} else if (memo !== id.toString()) { | ||
logger.debug( | ||
`Transaction memo matches donation memo for donation ID ${donation.id}`, | ||
); | ||
return; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add memo validation for 'create_account' transactions.
The memo validation is only performed for 'payment' type transactions. To prevent transaction conflicts, add similar validation for 'create_account' type.
if (transaction.type === 'payment') {
if (toWalletMemo) {
if (memo !== toWalletMemo) {
logger.debug(
`Transaction memo does not match donation memo for donation ID ${donation.id}`,
);
return;
}
} else if (typeof memo !== 'string' || memo !== id.toString()) {
logger.debug(
`Transaction memo does not match donation ID for donation ID ${donation.id}`,
);
return;
}
+ } else if (transaction.type === 'create_account') {
+ if (toWalletMemo) {
+ if (memo !== toWalletMemo) {
+ logger.debug(
+ `Transaction memo does not match donation memo for donation ID ${donation.id}`,
+ );
+ return;
+ }
+ } else if (typeof memo !== 'string' || memo !== id.toString()) {
+ logger.debug(
+ `Transaction memo does not match donation ID for donation ID ${donation.id}`,
+ );
+ return;
+ }
}
📝 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.
if (transaction.type === 'payment') { | |
if (toWalletMemo) { | |
if (memo !== toWalletMemo) { | |
logger.debug( | |
`Transaction memo does not match donation memo for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} else if (memo !== id.toString()) { | |
logger.debug( | |
`Transaction memo matches donation memo for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} | |
if (transaction.type === 'payment') { | |
if (toWalletMemo) { | |
if (memo !== toWalletMemo) { | |
logger.debug( | |
`Transaction memo does not match donation memo for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} else if (typeof memo !== 'string' || memo !== id.toString()) { | |
logger.debug( | |
`Transaction memo does not match donation ID for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} else if (transaction.type === 'create_account') { | |
if (toWalletMemo) { | |
if (memo !== toWalletMemo) { | |
logger.debug( | |
`Transaction memo does not match donation memo for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} else if (typeof memo !== 'string' || memo !== id.toString()) { | |
logger.debug( | |
`Transaction memo does not match donation ID for donation ID ${donation.id}`, | |
); | |
return; | |
} | |
} |
const memo = transaction.transaction.memo; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add null check for transaction memo.
The memo value should be validated before use to prevent potential runtime errors.
- const memo = transaction.transaction.memo;
+ const memo = transaction.transaction?.memo;
+ if (memo === undefined || memo === null) {
+ logger.debug(
+ `Transaction memo is missing for donation ID ${donation.id}`,
+ );
+ return;
+ }
📝 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.
const memo = transaction.transaction.memo; | |
const memo = transaction.transaction?.memo; | |
if (memo === undefined || memo === null) { | |
logger.debug( | |
`Transaction memo is missing for donation ID ${donation.id}`, | |
); | |
return; | |
} |
This fix, will be eliminate the transaction conflict only for muxed Stallar addresses (doesn't have a memo)
Summary by CodeRabbit
No user interface changes are expected with this update.