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.

Sign up for GitHub

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

Jump to bottom

Fix go private window behavior for exchange address #1426

Closed
wants to merge 4 commits into from
Closed

Fix go private window behavior for exchange address #1426

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e254498
Fix go private window behavore for exchange address
levoncrypto Apr 2, 2024
98dacc3
Add setting for <Make this a private transaction> window
levoncrypto Apr 2, 2024
7ca765c
Changes for optimize
levoncrypto Apr 5, 2024
f897199
Build fix
levoncrypto Apr 5, 2024
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
25 changes: 19 additions & 6 deletions src/qt/sendcoinsdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,25 @@ void SendCoinsDialog::on_sendButton_clicked()
}
} else if ((fAnonymousMode == false) && (sparkAddressCount == 0)) {
if (spark::IsSparkAllowed()) {
SendGoPrivateDialog goPrivateDialog;
bool clickedButton = goPrivateDialog.getClickedButton();
if (!clickedButton) {
setAnonymizeMode(true);
fNewRecipientAllowed = true;
return;
bool openPageTag = true;
for(int i = 0; i < recipients.size(); ++i){
std::string address = recipients[i].address.toStdString();
CBitcoinAddress add(address);
CTxDestination dest = add.Get();

if (boost::get<CExchangeKeyID>(&dest)) {
openPageTag = false;
}
}

if (openPageTag) {
SendGoPrivateDialog goPrivateDialog;
bool clickedButton = goPrivateDialog.getClickedButton();
if (!clickedButton) {
setAnonymizeMode(true);
fNewRecipientAllowed = true;
return;
}
Copy link

Choose a reason for hiding this comment

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

The logic to check for CExchangeKeyID type addresses and conditionally display the SendGoPrivateDialog is a crucial addition for enhancing the application's handling of private transactions. However, there are a few considerations and potential improvements:

  1. Error Handling and User Feedback: When the openPageTag is set to false due to the presence of a CExchangeKeyID type address, the function returns without any user feedback. It might be beneficial to inform the user why the operation cannot proceed in this case.

  2. Code Duplication: The pattern of iterating over recipients to perform checks appears multiple times in this function. Consider refactoring this pattern into a separate method to improve code maintainability and reduce duplication.

  3. Performance Consideration: The current implementation iterates over all recipients for each condition check. If the list of recipients is large, this could impact performance. Consider optimizing this by breaking early if possible or consolidating checks.

  4. User Experience: The decision to block the transaction entirely if any recipient address is of the CExchangeKeyID type might be too restrictive. It could be worth exploring alternatives that allow the user to proceed with the transaction for non-exchange addresses or provide a clear path to rectify the issue.

-                    setAnonymizeMode(true);
-                    fNewRecipientAllowed = true;
+                    // Consider providing user feedback here to explain why the transaction cannot proceed.
+                    QMessageBox::warning(this, tr("Incompatible Address"), tr("One or more recipient addresses are exchange addresses, which are not compatible with private transactions."));

Consider refactoring the recipient address type checks into a separate method to improve code readability and maintainability.

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.

Suggested change
bool openPageTag = true;
for(int i = 0; i < recipients.size(); ++i){
std::string address = recipients[i].address.toStdString();
CBitcoinAddress add(address);
CTxDestination dest = add.Get();
if (boost::get<CExchangeKeyID>(&dest)) {
openPageTag = false;
}
}
if (openPageTag) {
SendGoPrivateDialog goPrivateDialog;
bool clickedButton = goPrivateDialog.getClickedButton();
if (!clickedButton) {
setAnonymizeMode(true);
fNewRecipientAllowed = true;
return;
}
bool openPageTag = true;
for(int i = 0; i < recipients.size(); ++i){
std::string address = recipients[i].address.toStdString();
CBitcoinAddress add(address);
CTxDestination dest = add.Get();
if (boost::get<CExchangeKeyID>(&dest)) {
openPageTag = false;
}
}
if (openPageTag) {
SendGoPrivateDialog goPrivateDialog;
bool clickedButton = goPrivateDialog.getClickedButton();
if (!clickedButton) {
// Consider providing user feedback here to explain why the transaction cannot proceed.
QMessageBox::warning(this, tr("Incompatible Address"), tr("One or more recipient addresses are exchange addresses, which are not compatible with private transactions."));
return;
}

}
}
prepareStatus = model->prepareTransaction(currentTransaction, &ctrl);
Expand Down
Loading