Bitcoin-Qt: fix known addressbook bugs

- add qSort() for cachedAddressTable, as qLowerBound() and qUpperBound()
  require the list to be in ascending order (see
  http://harmattan-dev.nokia.com/docs/library/html/qt4/qtalgorithms.html#qLowerBound)
- add a new check in AddressTableModel::setData() to just return, when no
  changes were made to a label or an address (prevents entry duplication
  issue)
- remove "rec->label = value.toString();" from
  AddressTableModel::setData() as the label gets updated by
  AddressTablePriv::updateEntry() anyway (seems @sipa added this line via
  1025440184 (L6R225))
- add another new check in AddressTableModel::setData() to just return, if
  a duplicate address was found (prevents address overwrite)
- add a new check to EditAddressDialog::setModel() to prevent setting an
  invalid model
- re-work the switch-case statement in AddressTableModel::accept() to
  always break (as return get's called anyway) and order the list to match
  the enum definition
- make accept() in editaddressdialog.h a public slot, which it should be
- misc small coding style changes
This commit is contained in:
Philip Kaufmann 2013-01-08 08:17:58 +01:00
parent 429915bd0d
commit e6d2300562
4 changed files with 64 additions and 33 deletions

View file

@ -69,6 +69,8 @@ public:
QString::fromStdString(address.ToString()))); QString::fromStdString(address.ToString())));
} }
} }
// qLowerBound() and qUpperBound() require our cachedAddressTable list to be sorted in asc order
qSort(cachedAddressTable.begin(), cachedAddressTable.end(), AddressTableEntryLessThan());
} }
void updateEntry(const QString &address, const QString &label, bool isMine, int status) void updateEntry(const QString &address, const QString &label, bool isMine, int status)
@ -208,7 +210,7 @@ QVariant AddressTableModel::data(const QModelIndex &index, int role) const
return QVariant(); return QVariant();
} }
bool AddressTableModel::setData(const QModelIndex & index, const QVariant & value, int role) bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, int role)
{ {
if(!index.isValid()) if(!index.isValid())
return false; return false;
@ -221,18 +223,36 @@ bool AddressTableModel::setData(const QModelIndex & index, const QVariant & valu
switch(index.column()) switch(index.column())
{ {
case Label: case Label:
// Do nothing, if old label == new label
if(rec->label == value.toString())
{
editStatus = NO_CHANGES;
return false;
}
wallet->SetAddressBookName(CBitcoinAddress(rec->address.toStdString()).Get(), value.toString().toStdString()); wallet->SetAddressBookName(CBitcoinAddress(rec->address.toStdString()).Get(), value.toString().toStdString());
rec->label = value.toString();
break; break;
case Address: case Address:
// Do nothing, if old address == new address
if(CBitcoinAddress(rec->address.toStdString()) == CBitcoinAddress(value.toString().toStdString()))
{
editStatus = NO_CHANGES;
return false;
}
// Refuse to set invalid address, set error status and return false // Refuse to set invalid address, set error status and return false
if(!walletModel->validateAddress(value.toString())) else if(!walletModel->validateAddress(value.toString()))
{ {
editStatus = INVALID_ADDRESS; editStatus = INVALID_ADDRESS;
return false; return false;
} }
// Check for duplicate addresses to prevent accidental deletion of addresses, if you try
// to paste an existing address over another address (with a different label)
else if(wallet->mapAddressBook.count(CBitcoinAddress(value.toString().toStdString()).Get()))
{
editStatus = DUPLICATE_ADDRESS;
return false;
}
// Double-check that we're not overwriting a receiving address // Double-check that we're not overwriting a receiving address
if(rec->type == AddressTableEntry::Sending) else if(rec->type == AddressTableEntry::Sending)
{ {
{ {
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
@ -244,7 +264,6 @@ bool AddressTableModel::setData(const QModelIndex & index, const QVariant & valu
} }
break; break;
} }
return true; return true;
} }
return false; return false;
@ -262,7 +281,7 @@ QVariant AddressTableModel::headerData(int section, Qt::Orientation orientation,
return QVariant(); return QVariant();
} }
Qt::ItemFlags AddressTableModel::flags(const QModelIndex & index) const Qt::ItemFlags AddressTableModel::flags(const QModelIndex &index) const
{ {
if(!index.isValid()) if(!index.isValid())
return 0; return 0;
@ -279,7 +298,7 @@ Qt::ItemFlags AddressTableModel::flags(const QModelIndex & index) const
return retval; return retval;
} }
QModelIndex AddressTableModel::index(int row, int column, const QModelIndex & parent) const QModelIndex AddressTableModel::index(int row, int column, const QModelIndex &parent) const
{ {
Q_UNUSED(parent); Q_UNUSED(parent);
AddressTableEntry *data = priv->index(row); AddressTableEntry *data = priv->index(row);
@ -345,6 +364,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
{ {
return QString(); return QString();
} }
// Add entry // Add entry
{ {
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
@ -353,7 +373,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
return QString::fromStdString(strAddress); return QString::fromStdString(strAddress);
} }
bool AddressTableModel::removeRows(int row, int count, const QModelIndex & parent) bool AddressTableModel::removeRows(int row, int count, const QModelIndex &parent)
{ {
Q_UNUSED(parent); Q_UNUSED(parent);
AddressTableEntry *rec = priv->index(row); AddressTableEntry *rec = priv->index(row);

View file

@ -29,26 +29,27 @@ public:
/** Return status of edit/insert operation */ /** Return status of edit/insert operation */
enum EditStatus { enum EditStatus {
OK, OK, /**< Everything ok */
INVALID_ADDRESS, /**< Unparseable address */ NO_CHANGES, /**< No changes were made during edit operation */
DUPLICATE_ADDRESS, /**< Address already in address book */ INVALID_ADDRESS, /**< Unparseable address */
WALLET_UNLOCK_FAILURE, /**< Wallet could not be unlocked to create new receiving address */ DUPLICATE_ADDRESS, /**< Address already in address book */
KEY_GENERATION_FAILURE /**< Generating a new public key for a receiving address failed */ WALLET_UNLOCK_FAILURE, /**< Wallet could not be unlocked to create new receiving address */
KEY_GENERATION_FAILURE /**< Generating a new public key for a receiving address failed */
}; };
static const QString Send; /**< Specifies send address */ static const QString Send; /**< Specifies send address */
static const QString Receive; /**< Specifies receive address */ static const QString Receive; /**< Specifies receive address */
/** @name Methods overridden from QAbstractTableModel /** @name Methods overridden from QAbstractTableModel
@{*/ @{*/
int rowCount(const QModelIndex &parent) const; int rowCount(const QModelIndex &parent) const;
int columnCount(const QModelIndex &parent) const; int columnCount(const QModelIndex &parent) const;
QVariant data(const QModelIndex &index, int role) const; QVariant data(const QModelIndex &index, int role) const;
bool setData(const QModelIndex & index, const QVariant & value, int role); bool setData(const QModelIndex &index, const QVariant &value, int role);
QVariant headerData(int section, Qt::Orientation orientation, int role) const; QVariant headerData(int section, Qt::Orientation orientation, int role) const;
QModelIndex index(int row, int column, const QModelIndex & parent) const; QModelIndex index(int row, int column, const QModelIndex &parent) const;
bool removeRows(int row, int count, const QModelIndex & parent = QModelIndex()); bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex());
Qt::ItemFlags flags(const QModelIndex & index) const; Qt::ItemFlags flags(const QModelIndex &index) const;
/*@}*/ /*@}*/
/* Add an address to the model. /* Add an address to the model.

View file

@ -25,7 +25,7 @@ EditAddressDialog::EditAddressDialog(Mode mode, QWidget *parent) :
break; break;
case EditReceivingAddress: case EditReceivingAddress:
setWindowTitle(tr("Edit receiving address")); setWindowTitle(tr("Edit receiving address"));
ui->addressEdit->setDisabled(true); ui->addressEdit->setEnabled(false);
break; break;
case EditSendingAddress: case EditSendingAddress:
setWindowTitle(tr("Edit sending address")); setWindowTitle(tr("Edit sending address"));
@ -44,6 +44,9 @@ EditAddressDialog::~EditAddressDialog()
void EditAddressDialog::setModel(AddressTableModel *model) void EditAddressDialog::setModel(AddressTableModel *model)
{ {
this->model = model; this->model = model;
if(!model)
return;
mapper->setModel(model); mapper->setModel(model);
mapper->addMapping(ui->labelEdit, AddressTableModel::Label); mapper->addMapping(ui->labelEdit, AddressTableModel::Label);
mapper->addMapping(ui->addressEdit, AddressTableModel::Address); mapper->addMapping(ui->addressEdit, AddressTableModel::Address);
@ -58,6 +61,7 @@ bool EditAddressDialog::saveCurrentRow()
{ {
if(!model) if(!model)
return false; return false;
switch(mode) switch(mode)
{ {
case NewReceivingAddress: case NewReceivingAddress:
@ -82,35 +86,39 @@ void EditAddressDialog::accept()
{ {
if(!model) if(!model)
return; return;
if(!saveCurrentRow()) if(!saveCurrentRow())
{ {
switch(model->getEditStatus()) switch(model->getEditStatus())
{ {
case AddressTableModel::DUPLICATE_ADDRESS: case AddressTableModel::OK:
QMessageBox::warning(this, windowTitle(), // Failed with unknown reason. Just reject.
tr("The entered address \"%1\" is already in the address book.").arg(ui->addressEdit->text()), break;
QMessageBox::Ok, QMessageBox::Ok); case AddressTableModel::NO_CHANGES:
// No changes were made during edit operation. Just reject.
break; break;
case AddressTableModel::INVALID_ADDRESS: case AddressTableModel::INVALID_ADDRESS:
QMessageBox::warning(this, windowTitle(), QMessageBox::warning(this, windowTitle(),
tr("The entered address \"%1\" is not a valid Bitcoin address.").arg(ui->addressEdit->text()), tr("The entered address \"%1\" is not a valid Bitcoin address.").arg(ui->addressEdit->text()),
QMessageBox::Ok, QMessageBox::Ok); QMessageBox::Ok, QMessageBox::Ok);
return; break;
case AddressTableModel::DUPLICATE_ADDRESS:
QMessageBox::warning(this, windowTitle(),
tr("The entered address \"%1\" is already in the address book.").arg(ui->addressEdit->text()),
QMessageBox::Ok, QMessageBox::Ok);
break;
case AddressTableModel::WALLET_UNLOCK_FAILURE: case AddressTableModel::WALLET_UNLOCK_FAILURE:
QMessageBox::critical(this, windowTitle(), QMessageBox::critical(this, windowTitle(),
tr("Could not unlock wallet."), tr("Could not unlock wallet."),
QMessageBox::Ok, QMessageBox::Ok); QMessageBox::Ok, QMessageBox::Ok);
return; break;
case AddressTableModel::KEY_GENERATION_FAILURE: case AddressTableModel::KEY_GENERATION_FAILURE:
QMessageBox::critical(this, windowTitle(), QMessageBox::critical(this, windowTitle(),
tr("New key generation failed."), tr("New key generation failed."),
QMessageBox::Ok, QMessageBox::Ok); QMessageBox::Ok, QMessageBox::Ok);
return;
case AddressTableModel::OK:
// Failed with unknown reason. Just reject.
break; break;
}
}
return; return;
} }
QDialog::accept(); QDialog::accept();

View file

@ -27,15 +27,17 @@ public:
}; };
explicit EditAddressDialog(Mode mode, QWidget *parent = 0); explicit EditAddressDialog(Mode mode, QWidget *parent = 0);
~EditAddressDialog(); ~EditAddressDialog();
void setModel(AddressTableModel *model); void setModel(AddressTableModel *model);
void loadRow(int row); void loadRow(int row);
void accept();
QString getAddress() const; QString getAddress() const;
void setAddress(const QString &address); void setAddress(const QString &address);
public slots:
void accept();
private: private:
bool saveCurrentRow(); bool saveCurrentRow();