-
-
Notifications
You must be signed in to change notification settings - Fork 355
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 warning messages with sending #1434
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe recent changes focus on enhancing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SendCoinsEntry
participant WarningGenerator
User->>SendCoinsEntry: setfAnonymousMode(true)
SendCoinsEntry->>SendCoinsEntry: Update fAnonymousMode
User->>SendCoinsEntry: setWarning(true)
SendCoinsEntry->>WarningGenerator: generateWarningText(address, true)
WarningGenerator-->>SendCoinsEntry: WarningText
SendCoinsEntry->>SendCoinsEntry: Set Warning Text and Visibility
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 3
src/qt/sendcoinsentry.h
Outdated
@@ -48,8 +48,9 @@ class SendCoinsEntry : public QStackedWidget | |||
QWidget *setupTabChain(QWidget *prev); | |||
|
|||
void setFocus(); | |||
void setWarning(bool fAnonymousMode); | |||
|
|||
void setWarning(const bool fAnonymousMode); |
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.
Ensure the setWarning
method parameter is consistently const
.
- void setWarning(const bool fAnonymousMode);
+ void setWarning(bool fAnonymousMode);
This change aligns the method signature with the typical usage of primitive types where const
is not usually beneficial for parameters passed by value.
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.
void setWarning(const bool fAnonymousMode); | |
void setWarning(bool fAnonymousMode); |
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.
agree with bot, please revert this change
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.
@levonpetrosyan93, thank you for your feedback. I'll revert the change to remove the const
qualifier from the setWarning
method parameter as you agreed. This aligns with typical C++ practices for parameters passed by value.
src/qt/sendcoinsentry.h
Outdated
@@ -71,6 +72,7 @@ private Q_SLOTS: | |||
WalletModel *model; | |||
const PlatformStyle *platformStyle; | |||
bool isPcodeEntry; | |||
bool fAnonymousMode; |
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.
Initialization of fAnonymousMode
should be explicit.
- bool fAnonymousMode;
+ bool fAnonymousMode = false;
It's a good practice to initialize member variables explicitly to avoid undefined behavior.
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.
bool fAnonymousMode; | |
bool fAnonymousMode = false; |
src/qt/sendcoinsentry.cpp
Outdated
QString address = ui->payTo->text(); | ||
QString warningText; | ||
|
||
if (address.startsWith("EX")) { | ||
warningText = tr("You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address."); | ||
} else { | ||
ui->textWarning->show(); | ||
ui->iconWarning->show(); | ||
if (!fAnonymousMode) { | ||
if (model->validateAddress(address)) { | ||
warningText = tr("You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead."); | ||
} else if (model->validateSparkAddress(address)) { | ||
warningText = tr("You are sending Firo from a transparent address to a Spark address."); | ||
} | ||
} else { | ||
if (model->validateAddress(address)) { | ||
warningText = tr("You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits."); | ||
} else if (model->validateSparkAddress(address)) { | ||
warningText = tr("You are sending Firo from a Spark address to another Spark address. This transaction is fully private."); | ||
} | ||
} | ||
} | ||
|
||
const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address); | ||
ui->textWarning->setText(warningText); | ||
ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | ||
ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress); |
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 setWarning
to reduce complexity.
- void SendCoinsEntry::setWarning(bool fAnonymousMode) {
- QString address = ui->payTo->text();
- QString warningText;
- ...
- ui->textWarning->setText(warningText);
- ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
- ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
- }
+ void SendCoinsEntry::setWarning(bool fAnonymousMode) {
+ const QString address = ui->payTo->text();
+ const QString warningText = generateWarningText(address, fAnonymousMode);
+ const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address);
+ ui->textWarning->setText(warningText);
+ ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
+ ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
+ }
+ QString SendCoinsEntry::generateWarningText(const QString& address, bool fAnonymousMode) {
+ // Implementation of warning text generation based on address and mode
+ }
This refactor suggestion aims to reduce the complexity of the setWarning
method by separating the warning text generation into a new method. This makes the code cleaner and easier to maintain.
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.
QString address = ui->payTo->text(); | |
QString warningText; | |
if (address.startsWith("EX")) { | |
warningText = tr("You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address."); | |
} else { | |
ui->textWarning->show(); | |
ui->iconWarning->show(); | |
if (!fAnonymousMode) { | |
if (model->validateAddress(address)) { | |
warningText = tr("You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead."); | |
} else if (model->validateSparkAddress(address)) { | |
warningText = tr("You are sending Firo from a transparent address to a Spark address."); | |
} | |
} else { | |
if (model->validateAddress(address)) { | |
warningText = tr("You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits."); | |
} else if (model->validateSparkAddress(address)) { | |
warningText = tr("You are sending Firo from a Spark address to another Spark address. This transaction is fully private."); | |
} | |
} | |
} | |
const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address); | |
ui->textWarning->setText(warningText); | |
ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
void SendCoinsEntry::setWarning(bool fAnonymousMode) { | |
const QString address = ui->payTo->text(); | |
const QString warningText = generateWarningText(address, fAnonymousMode); | |
const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address); | |
ui->textWarning->setText(warningText); | |
ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
} | |
QString SendCoinsEntry::generateWarningText(const QString& address, bool fAnonymousMode) { | |
// Implementation of warning text generation based on address and mode | |
} |
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: 1
src/qt/sendcoinsentry.cpp
Outdated
QString SendCoinsEntry::generateWarningText(const QString& address, const bool fAnonymousMode) | ||
{ | ||
if(fAnonymousMode) { | ||
ui->textWarning->hide(); | ||
ui->iconWarning->hide(); | ||
QString warningText; | ||
|
||
if (address.startsWith("EX")) { | ||
warningText = tr("You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address."); | ||
} else { | ||
ui->textWarning->show(); | ||
ui->iconWarning->show(); | ||
if (!fAnonymousMode) { | ||
if (model->validateAddress(address)) { | ||
warningText = tr("You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead."); | ||
} else if (model->validateSparkAddress(address)) { | ||
warningText = tr("You are sending Firo from a transparent address to a Spark address."); | ||
} | ||
} else { | ||
if (model->validateAddress(address)) { | ||
warningText = tr("You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits."); | ||
} else if (model->validateSparkAddress(address)) { | ||
warningText = tr("You are sending Firo from a Spark address to another Spark address. This transaction is fully private."); | ||
} | ||
} | ||
} | ||
return warningText; |
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.
Implementation of generateWarningText
method appears comprehensive, handling various scenarios based on address type and anonymity mode.
Would you like me to help with writing unit tests for this method to ensure all edge cases are covered?
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.
Yes
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.
@reubenyap, here are the unit tests for the generateWarningText
method in the SendCoinsEntry
class. These tests cover various scenarios based on the address type and anonymity mode:
#include <QtTest>
#include "sendcoinsentry.h"
#include "walletmodel.h"
class TestSendCoinsEntry : public QObject
{
Q_OBJECT
private slots:
void testGenerateWarningText();
};
void TestSendCoinsEntry::testGenerateWarningText()
{
SendCoinsEntry entry;
WalletModel model; // Mocked model
entry.setModel(&model);
// Test sending to an Exchange Address
QCOMPARE(entry.generateWarningText("EX123", false), QObject::tr("You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address."));
// Test sending from a transparent address to another transparent address
model.setAddressType(false); // Mock method to set address type validation
QCOMPARE(entry.generateWarningText("1DUMMY", false), QObject::tr("You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead."));
// Test sending from a transparent address to a Spark address
model.setAddressType(true); // Mock method to set Spark address validation
QCOMPARE(entry.generateWarningText("SPARK123", false), QObject::tr("You are sending Firo from a transparent address to a Spark address."));
// Test sending from a Spark address to another Spark address
QCOMPARE(entry.generateWarningText("SPARK123", true), QObject::tr("You are sending Firo from a Spark address to another Spark address. This transaction is fully private."));
// Test sending from a private Spark pool to a transparent address
model.setAddressType(false); // Mock method to set address type validation
QCOMPARE(entry.generateWarningText("1DUMMY", true), QObject::tr("You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits."));
}
QTEST_MAIN(TestSendCoinsEntry)
#include "test_sendcoinsentry.moc"
These tests assume the existence of methods in WalletModel
to set and validate address types, which you might need to mock or implement accordingly. Let me know if you need further assistance or modifications to these tests!
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: 2
void SendCoinsEntry::setWarning(bool fAnonymousMode) { | ||
const QString address = ui->payTo->text(); | ||
const QString warningText = generateWarningText(address, fAnonymousMode); | ||
const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address); | ||
ui->textWarning->setText(warningText); | ||
ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | ||
ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | ||
} |
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 setWarning
to reduce complexity.
- void SendCoinsEntry::setWarning(bool fAnonymousMode) {
- const QString address = ui->payTo->text();
- const QString warningText = generateWarningText(address, fAnonymousMode);
- const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address);
- ui->textWarning->setText(warningText);
- ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
- ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
- }
+ void SendCoinsEntry::setWarning(bool fAnonymousMode) {
+ const QString address = ui->payTo->text();
+ const QString warningText = generateWarningText(address, fAnonymousMode);
+ const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address);
+ ui->textWarning->setText(warningText);
+ ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
+ ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress);
+ }
+ QString SendCoinsEntry::generateWarningText(const QString& address, bool fAnonymousMode) {
+ // Implementation of warning text generation based on address and mode
+ }
This refactor suggestion aims to reduce the complexity of the setWarning
method by separating the warning text generation into a new method. This makes the code cleaner and easier to maintain.
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.
void SendCoinsEntry::setWarning(bool fAnonymousMode) { | |
const QString address = ui->payTo->text(); | |
const QString warningText = generateWarningText(address, fAnonymousMode); | |
const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address); | |
ui->textWarning->setText(warningText); | |
ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
} | |
void SendCoinsEntry::setWarning(bool fAnonymousMode) { | |
const QString address = ui->payTo->text(); | |
const QString warningText = generateWarningText(address, fAnonymousMode); | |
const bool hasValidAddress = model->validateAddress(address) || model->validateSparkAddress(address); | |
ui->textWarning->setText(warningText); | |
ui->textWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
ui->iconWarning->setVisible(!warningText.isEmpty() && hasValidAddress); | |
} | |
QString SendCoinsEntry::generateWarningText(const QString& address, bool fAnonymousMode) { | |
// Implementation of warning text generation based on address and mode | |
} |
src/qt/sendcoinsentry.cpp
Outdated
QString SendCoinsEntry::generateWarningText(const QString& address, const bool fAnonymousMode) | ||
{ | ||
if(fAnonymousMode) { | ||
ui->textWarning->hide(); | ||
ui->iconWarning->hide(); | ||
QString warningText; | ||
|
||
if (address.startsWith("EX")) { | ||
warningText = tr(" You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address."); | ||
} else { | ||
ui->textWarning->show(); | ||
ui->iconWarning->show(); | ||
if (!fAnonymousMode) { | ||
if (model->validateAddress(address)) { | ||
warningText = tr(" You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead."); | ||
} else if (model->validateSparkAddress(address)) { | ||
warningText = tr(" You are sending Firo from a transparent address to a Spark address."); | ||
} | ||
} else { | ||
if (model->validateSparkAddress(address)) { | ||
warningText = tr(" You are sending Firo from a Spark address to another Spark address. This transaction is fully private."); | ||
} else if (model->validateAddress(address)) { | ||
warningText = tr(" You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits."); | ||
} | ||
} | ||
} | ||
return warningText; |
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.
Implementation of generateWarningText
method appears comprehensive, handling various scenarios based on address type and anonymity mode.
Would you like me to help with writing unit tests for this method to ensure all edge cases are covered?
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.
Yes
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.
@reubenyap, here are the unit tests for the generateWarningText
method in the SendCoinsEntry
class. These tests cover various scenarios based on the address type and anonymity mode:
#include <gtest/gtest.h>
#include "sendcoinsentry.h"
#include "walletmodel.h"
class SendCoinsEntryTest : public ::testing::Test {
protected:
SendCoinsEntry *entry;
WalletModel *model;
void SetUp() override {
entry = new SendCoinsEntry(nullptr, nullptr);
model = new WalletModel(nullptr, nullptr);
entry->setModel(model);
}
void TearDown() override {
delete entry;
delete model;
}
};
TEST_F(SendCoinsEntryTest, TestExchangeAddress) {
entry->setfAnonymousMode(false);
QString address = "EX123";
QString expected = "You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address.";
EXPECT_EQ(entry->generateWarningText(address, false), expected);
}
TEST_F(SendCoinsEntryTest, TestTransparentToTransparent) {
entry->setfAnonymousMode(false);
QString address = "1BitcoinAddr...";
QString expected = "You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead.";
EXPECT_EQ(entry->generateWarningText(address, false), expected);
}
TEST_F(SendCoinsEntryTest, TestTransparentToSpark) {
entry->setfAnonymousMode(false);
QString address = "SparkAddr...";
QString expected = "You are sending Firo from a transparent address to a Spark address.";
EXPECT_EQ(entry->generateWarningText(address, false), expected);
}
TEST_F(SendCoinsEntryTest, TestSparkToSpark) {
entry->setfAnonymousMode(true);
QString address = "SparkAddr...";
QString expected = "You are sending Firo from a Spark address to another Spark address. This transaction is fully private.";
EXPECT_EQ(entry->generateWarningText(address, true), expected);
}
TEST_F(SendCoinsEntryTest, TestInvalidAddress) {
entry->setfAnonymousMode(false);
QString address = "InvalidAddr";
QString expected = "";
EXPECT_EQ(entry->generateWarningText(address, false), expected);
}
These tests should help ensure that the generateWarningText
method behaves as expected under different conditions. Let me know if there are any other scenarios you would like to include or if there's anything else I can assist with!
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: 1
src/qt/test/test_sendcoinsentry.cpp
Outdated
void TestSendCoinsEntry::testGenerateWarningText() | ||
{ | ||
SendCoinsEntry* entry; | ||
QCOMPARE(entry->generateWarningText("EXRSxX8yJHudk4QswGf3N5aPVTUi5Q1ZdX56", false), QObject::tr(" You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address.")); | ||
QCOMPARE(entry->generateWarningText("TLyNUvysvUyt2u6vL74NEkB6ed8LTQd3mz", false), QObject::tr(" You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead.")); | ||
QCOMPARE(entry->generateWarningText("sr1ek2uspg2v4qu0lmccrnj90tfkdpp5zmpykr4ffdprqlf0s4devl8n0674s4d4cthxsa5w9p66s5x0zgw982t80xx9uzmxysxuawmupgfa0xecj9shm6pj7l3rshqxqtg94k88fg5u856r", false), QObject::tr(" You are sending Firo from a transparent address to a Spark address.")); | ||
QCOMPARE(entry->generateWarningText("sr1ek2uspg2v4qu0lmccrnj90tfkdpp5zmpykr4ffdprqlf0s4devl8n0674s4d4cthxsa5w9p66s5x0zgw982t80xx9uzmxysxuawmupgfa0xecj9shm6pj7l3rshqxqtg94k88fg5u856r", true), QObject::tr(" You are sending Firo from a Spark address to another Spark address. This transaction is fully private.")); | ||
QCOMPARE(entry->generateWarningText("TLyNUvysvUyt2u6vL74NEkB6ed8LTQd3mz", true), QObject::tr(" You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits.")); | ||
} |
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.
The entry
pointer is used uninitialized, which can lead to undefined behavior or crashes during testing.
- SendCoinsEntry* entry;
+ SendCoinsEntry* entry = new SendCoinsEntry();
+ // Remember to delete `entry` at the end of the test to avoid memory leaks.
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.
void TestSendCoinsEntry::testGenerateWarningText() | |
{ | |
SendCoinsEntry* entry; | |
QCOMPARE(entry->generateWarningText("EXRSxX8yJHudk4QswGf3N5aPVTUi5Q1ZdX56", false), QObject::tr(" You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address.")); | |
QCOMPARE(entry->generateWarningText("TLyNUvysvUyt2u6vL74NEkB6ed8LTQd3mz", false), QObject::tr(" You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead.")); | |
QCOMPARE(entry->generateWarningText("sr1ek2uspg2v4qu0lmccrnj90tfkdpp5zmpykr4ffdprqlf0s4devl8n0674s4d4cthxsa5w9p66s5x0zgw982t80xx9uzmxysxuawmupgfa0xecj9shm6pj7l3rshqxqtg94k88fg5u856r", false), QObject::tr(" You are sending Firo from a transparent address to a Spark address.")); | |
QCOMPARE(entry->generateWarningText("sr1ek2uspg2v4qu0lmccrnj90tfkdpp5zmpykr4ffdprqlf0s4devl8n0674s4d4cthxsa5w9p66s5x0zgw982t80xx9uzmxysxuawmupgfa0xecj9shm6pj7l3rshqxqtg94k88fg5u856r", true), QObject::tr(" You are sending Firo from a Spark address to another Spark address. This transaction is fully private.")); | |
QCOMPARE(entry->generateWarningText("TLyNUvysvUyt2u6vL74NEkB6ed8LTQd3mz", true), QObject::tr(" You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits.")); | |
} | |
void TestSendCoinsEntry::testGenerateWarningText() | |
{ | |
SendCoinsEntry* entry = new SendCoinsEntry(); | |
// Remember to delete `entry` at the end of the test to avoid memory leaks. | |
QCOMPARE(entry->generateWarningText("EXRSxX8yJHudk4QswGf3N5aPVTUi5Q1ZdX56", false), QObject::tr(" You are sending Firo to an Exchange Address. Exchange Addresses can only receive funds from a transparent address.")); | |
QCOMPARE(entry->generateWarningText("TLyNUvysvUyt2u6vL74NEkB6ed8LTQd3mz", false), QObject::tr(" You are sending Firo from a transparent address to another transparent address. To protect your privacy, we recommend using Spark addresses instead.")); | |
QCOMPARE(entry->generateWarningText("sr1ek2uspg2v4qu0lmccrnj90tfkdpp5zmpykr4ffdprqlf0s4devl8n0674s4d4cthxsa5w9p66s5x0zgw982t80xx9uzmxysxuawmupgfa0xecj9shm6pj7l3rshqxqtg94k88fg5u856r", false), QObject::tr(" You are sending Firo from a transparent address to a Spark address.")); | |
QCOMPARE(entry->generateWarningText("sr1ek2uspg2v4qu0lmccrnj90tfkdpp5zmpykr4ffdprqlf0s4devl8n0674s4d4cthxsa5w9p66s5x0zgw982t80xx9uzmxysxuawmupgfa0xecj9shm6pj7l3rshqxqtg94k88fg5u856r", true), QObject::tr(" You are sending Firo from a Spark address to another Spark address. This transaction is fully private.")); | |
QCOMPARE(entry->generateWarningText("TLyNUvysvUyt2u6vL74NEkB6ed8LTQd3mz", true), QObject::tr(" You are sending Firo from a private Spark pool to a transparent address. Please note that some exchanges do not accept direct Spark deposits.")); | |
} |
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: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/Makefile.qttest.include (4 hunks)
Additional comments not posted (4)
src/Makefile.qttest.include (4)
11-12
: Added new MOC files forsendcoinsentry
tests correctly.
21-22
: Added new header files forsendcoinsentry
tests correctly.
32-32
: Correctly addedtest_sendcoinsentry.cpp
to the sources list for compiling the new tests.
49-50
: Added multiple libraries to the linker dependencies. Ensure all added libraries are utilized to avoid unnecessary build overhead.
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: 0
Outside diff range and nitpick comments (1)
src/qt/sendcoinsentry.cpp (1)
14-14
: Consider removing the direct inclusion of wallet headers in UI code.Directly including wallet headers in UI components can lead to tighter coupling between the UI and the core logic. Consider using signals and slots or other decoupling mechanisms to maintain separation of concerns.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/qt/sendcoinsentry.cpp (4 hunks)
- src/qt/sendcoinsentry.h (2 hunks)
- src/qt/test/test_main.cpp (3 hunks)
- src/qt/test/test_sendcoinsentry.cpp (1 hunks)
- src/qt/test/uritests.cpp (1 hunks)
Additional comments not posted (7)
src/qt/test/test_sendcoinsentry.cpp (1)
3-10
: The test cases intestGenerateWarningText
are well-implemented and cover various scenarios for address types and anonymity modes.src/qt/test/test_main.cpp (1)
36-36
: The changes to useREGTEST
for chain parameters and the addition ofTestSendCoinsEntry
are appropriate for enhancing test coverage in a controlled environment.Also applies to: 52-53
src/qt/sendcoinsentry.h (1)
52-54
: The addition ofsetfAnonymousMode
andgenerateWarningText
, along with the initialization offAnonymousMode
tofalse
, correctly implements the new functionality and follows good coding practices.Also applies to: 76-76
src/qt/test/uritests.cpp (1)
54-54
: The update to the URI scheme fromfiro://
tofiro:
is appropriate and aligns with typical URI formats. The tests correctly validate the functionality of URI parsing.src/qt/sendcoinsentry.cpp (3)
127-134
: Refactor suggestion implemented successfully.The refactor of the
setWarning
method to utilizegenerateWarningText
for generating the warning message has been successfully implemented, enhancing modularity and maintainability.
257-260
: Proper implementation of thesetfAnonymousMode
method.The method
setfAnonymousMode
has been added to manage the anonymous mode state effectively. This change aligns with the PR's objectives to enhance the handling of warning messages based on the anonymity mode.
136-157
: Ensure comprehensive handling of address validation and warning generation.The
generateWarningText
method now handles various scenarios based on the address type and anonymity mode. Ensure that all edge cases are covered by unit tests, particularly for new address types or changes in address validation logic.Verification successful
The
generateWarningText
function is used in multiple places, and there are unit tests covering various scenarios. However, to ensure comprehensive handling of address validation and warning generation, we need to verify the unit tests and the address validation logic.
The unit tests for the
generateWarningText
function intest_sendcoinsentry.cpp
cover various scenarios, including sending to exchange addresses, transparent addresses, and Spark addresses, both in anonymous and non-anonymous modes. Additionally, the address validation functionsvalidateAddress
andvalidateSparkAddress
are used extensively throughout the codebase, ensuring comprehensive validation logic.
The unit tests in
test_sendcoinsentry.cpp
cover the following scenarios:
- Sending to an exchange address.
- Sending from a transparent address to another transparent address.
- Sending from a transparent address to a Spark address.
- Sending from a Spark address to another Spark address.
- Sending from a private Spark pool to a transparent address.
The address validation functions are defined in
wallet.h
andwallet.cpp
and are used in various parts of the codebase, includingsendcoinsentry.cpp
,sendcoinsdialog.cpp
, and other relevant files.Based on the provided context and the results from the shell scripts, it appears that the address validation and warning generation logic is comprehensive and well-tested.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all address types are handled correctly in generateWarningText. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type cpp $'generateWarningText'Length of output: 2036
Script:
#!/bin/bash # Description: Verify the unit tests for generateWarningText and check address validation logic. # Test: Display the unit tests for generateWarningText. rg --type cpp --context 5 'generateWarningText' src/qt/test/test_sendcoinsentry.cpp # Test: Check the address validation functions in the main codebase. rg --type cpp 'validateAddress|validateSparkAddress'Length of output: 5983
No description provided.