TransactionHistory: fix use-after-free bugs

This commit is contained in:
xiphon 2019-12-16 07:50:01 +00:00
parent 46227bdad0
commit 4e1f7349c4
5 changed files with 118 additions and 145 deletions

View file

@ -36,16 +36,18 @@
#include <QWriteLocker> #include <QWriteLocker>
TransactionInfo *TransactionHistory::transaction(int index) bool TransactionHistory::transaction(int index, std::function<void (TransactionInfo &)> callback)
{ {
QReadLocker locker(&m_tinfoLock); QReadLocker locker(&m_lock);
if (index < 0 || index >= m_tinfo.size()) { if (index < 0 || index >= m_tinfo.size()) {
qCritical("%s: no transaction info for index %d", __FUNCTION__, index); qCritical("%s: no transaction info for index %d", __FUNCTION__, index);
qCritical("%s: there's %d transactions in backend", __FUNCTION__, m_pimpl->count()); qCritical("%s: there's %d transactions in backend", __FUNCTION__, m_pimpl->count());
return nullptr; return false;
} }
return m_tinfo.at(index);
callback(*m_tinfo.value(index));
return true;
} }
//// XXX: not sure if this method really needed; //// XXX: not sure if this method really needed;
@ -54,7 +56,7 @@ TransactionInfo *TransactionHistory::transaction(int index)
// return nullptr; // return nullptr;
//} //}
QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const void TransactionHistory::refresh(quint32 accountIndex)
{ {
QDateTime firstDateTime = QDateTime(QDate(2014, 4, 18)); // the genesis block QDateTime firstDateTime = QDateTime(QDate(2014, 4, 18)); // the genesis block
QDateTime lastDateTime = QDateTime::currentDateTime().addDays(1); // tomorrow (guard against jitter and timezones) QDateTime lastDateTime = QDateTime::currentDateTime().addDays(1); // tomorrow (guard against jitter and timezones)
@ -62,23 +64,24 @@ QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
emit refreshStarted(); emit refreshStarted();
{ {
QWriteLocker locker(&m_tinfoLock); QWriteLocker locker(&m_lock);
// XXX this invalidates previously saved history that might be used by model
qDeleteAll(m_tinfo); qDeleteAll(m_tinfo);
m_tinfo.clear(); m_tinfo.clear();
quint64 lastTxHeight = 0; quint64 lastTxHeight = 0;
m_locked = false; m_locked = false;
m_minutesToUnlock = 0; m_minutesToUnlock = 0;
TransactionHistory * parent = const_cast<TransactionHistory*>(this);
m_pimpl->refresh();
for (const auto i : m_pimpl->getAll()) { for (const auto i : m_pimpl->getAll()) {
TransactionInfo * ti = new TransactionInfo(i, parent); if (i->subaddrAccount() != accountIndex) {
if (ti->subaddrAccount() != accountIndex) {
delete ti;
continue; continue;
} }
m_tinfo.append(ti);
m_tinfo.append(new TransactionInfo(i, this));
const TransactionInfo *ti = m_tinfo.back();
// looking for transactions timestamp scope // looking for transactions timestamp scope
if (ti->timestamp() >= lastDateTime) { if (ti->timestamp() >= lastDateTime) {
lastDateTime = ti->timestamp(); lastDateTime = ti->timestamp();
@ -94,7 +97,6 @@ QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
m_minutesToUnlock = (requiredConfirmations - ti->confirmations()) * 2; m_minutesToUnlock = (requiredConfirmations - ti->confirmations()) * 2;
m_locked = true; m_locked = true;
} }
} }
} }
@ -108,21 +110,11 @@ QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
m_lastDateTime = lastDateTime; m_lastDateTime = lastDateTime;
emit lastDateTimeChanged(); emit lastDateTimeChanged();
} }
return m_tinfo;
}
void TransactionHistory::refresh(quint32 accountIndex)
{
// rebuilding transaction list in wallet_api;
m_pimpl->refresh();
// copying list here and keep track on every item to avoid memleaks
getAll(accountIndex);
} }
quint64 TransactionHistory::count() const quint64 TransactionHistory::count() const
{ {
QReadLocker locker(&m_tinfoLock); QReadLocker locker(&m_lock);
return m_tinfo.count(); return m_tinfo.count();
} }
@ -157,11 +149,6 @@ TransactionHistory::TransactionHistory(Monero::TransactionHistory *pimpl, QObjec
QString TransactionHistory::writeCSV(quint32 accountIndex, QString out) QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
{ {
QList<TransactionInfo *> history = this->getAll(accountIndex);
if(history.count() < 1){
return QString("");
}
// construct filename // construct filename
qint64 now = QDateTime::currentDateTime().currentMSecsSinceEpoch(); qint64 now = QDateTime::currentDateTime().currentMSecsSinceEpoch();
QString fn = QString(QString("%1/monero-txs_%2.csv").arg(out, QString::number(now / 1000))); QString fn = QString(QString("%1/monero-txs_%2.csv").arg(out, QString::number(now / 1000)));
@ -176,15 +163,21 @@ QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
QTextStream output(&data); QTextStream output(&data);
output << "blockHeight,epoch,date,direction,amount,atomicAmount,fee,txid,label,subaddrAccount,paymentId\n"; output << "blockHeight,epoch,date,direction,amount,atomicAmount,fee,txid,label,subaddrAccount,paymentId\n";
foreach(const TransactionInfo *info, history) QReadLocker locker(&m_lock);
{ for (const auto &tx : m_pimpl->getAll()) {
if (tx->subaddrAccount() != accountIndex) {
continue;
}
TransactionInfo info(tx, this);
// collect column data // collect column data
double amount = info->amount(); double amount = info.amount();
quint64 atomicAmount = info->atomicAmount(); quint64 atomicAmount = info.atomicAmount();
quint32 subaddrAccount = info->subaddrAccount(); quint32 subaddrAccount = info.subaddrAccount();
QString fee = info->fee(); QString fee = info.fee();
QString direction = QString(""); QString direction = QString("");
TransactionInfo::Direction _direction = info->direction(); TransactionInfo::Direction _direction = info.direction();
if(_direction == TransactionInfo::Direction_In) if(_direction == TransactionInfo::Direction_In)
{ {
direction = QString("in"); direction = QString("in");
@ -195,14 +188,14 @@ QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
else { else {
continue; // skip TransactionInfo::Direction_Both continue; // skip TransactionInfo::Direction_Both
} }
QString label = info->label(); QString label = info.label();
label.remove(QChar('"')); // reserved label.remove(QChar('"')); // reserved
quint64 blockHeight = info->blockHeight(); quint64 blockHeight = info.blockHeight();
QDateTime timeStamp = info->timestamp(); QDateTime timeStamp = info.timestamp();
QString date = info->date() + " " + info->time(); QString date = info.date() + " " + info.time();
uint epoch = timeStamp.toTime_t(); uint epoch = timeStamp.toTime_t();
QString displayAmount = info->displayAmount(); QString displayAmount = info.displayAmount();
QString paymentId = info->paymentId(); QString paymentId = info.paymentId();
if(paymentId == "0000000000000000"){ if(paymentId == "0000000000000000"){
paymentId = ""; paymentId = "";
} }
@ -211,7 +204,7 @@ QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
QString line = QString("%1,%2,%3,%4,%5,%6,%7,%8,\"%9\",%10,%11\n") QString line = QString("%1,%2,%3,%4,%5,%6,%7,%8,\"%9\",%10,%11\n")
.arg(QString::number(blockHeight), QString::number(epoch), date) .arg(QString::number(blockHeight), QString::number(epoch), date)
.arg(direction, QString::number(amount), QString::number(atomicAmount)) .arg(direction, QString::number(amount), QString::number(atomicAmount))
.arg(info->fee(), info->hash(), label, QString::number(subaddrAccount)) .arg(info.fee(), info.hash(), label, QString::number(subaddrAccount))
.arg(paymentId); .arg(paymentId);
output << line; output << line;
} }

View file

@ -50,9 +50,8 @@ class TransactionHistory : public QObject
Q_PROPERTY(bool locked READ locked) Q_PROPERTY(bool locked READ locked)
public: public:
Q_INVOKABLE TransactionInfo *transaction(int index); Q_INVOKABLE bool transaction(int index, std::function<void (TransactionInfo &)> callback);
// Q_INVOKABLE TransactionInfo * transaction(const QString &id); // Q_INVOKABLE TransactionInfo * transaction(const QString &id);
Q_INVOKABLE QList<TransactionInfo*> getAll(quint32 accountIndex) const;
Q_INVOKABLE void refresh(quint32 accountIndex); Q_INVOKABLE void refresh(quint32 accountIndex);
Q_INVOKABLE QString writeCSV(quint32 accountIndex, QString out); Q_INVOKABLE QString writeCSV(quint32 accountIndex, QString out);
quint64 count() const; quint64 count() const;
@ -75,9 +74,9 @@ private:
private: private:
friend class Wallet; friend class Wallet;
mutable QReadWriteLock m_lock;
Monero::TransactionHistory * m_pimpl; Monero::TransactionHistory * m_pimpl;
mutable QList<TransactionInfo*> m_tinfo; mutable QList<TransactionInfo*> m_tinfo;
mutable QReadWriteLock m_tinfoLock;
mutable QDateTime m_firstDateTime; mutable QDateTime m_firstDateTime;
mutable QDateTime m_lastDateTime; mutable QDateTime m_lastDateTime;
mutable int m_minutesToUnlock; mutable int m_minutesToUnlock;

View file

@ -100,8 +100,4 @@ private:
mutable QList<Transfer*> m_transfers; mutable QList<Transfer*> m_transfers;
}; };
// in order to wrap it to QVariant
Q_DECLARE_METATYPE(TransactionInfo*)
#endif // TRANSACTIONINFO_H #endif // TRANSACTIONINFO_H

View file

@ -59,106 +59,90 @@ TransactionHistory *TransactionHistoryModel::transactionHistory() const
return m_transactionHistory; return m_transactionHistory;
} }
QVariant TransactionHistoryModel::parseTransactionInfo(const TransactionInfo &tInfo, int role) const
{
switch (role)
{
case TransactionDirectionRole:
return QVariant::fromValue(tInfo.direction());
case TransactionPendingRole:
return tInfo.isPending();
case TransactionFailedRole:
return tInfo.isFailed();
case TransactionAmountRole:
return tInfo.amount();
case TransactionDisplayAmountRole:
return tInfo.displayAmount();
case TransactionAtomicAmountRole:
return tInfo.atomicAmount();
case TransactionFeeRole:
return tInfo.fee();
case TransactionBlockHeightRole:
{
// Use NULL QVariant for transactions without height.
// Forces them to be displayed at top when sorted by blockHeight.
if (tInfo.blockHeight() != 0)
{
return tInfo.blockHeight();
}
return QVariant();
}
case TransactionSubaddrIndexRole:
{
QString str = QString{""};
bool first = true;
for (quint32 i : tInfo.subaddrIndex())
{
if (!first)
str += QString{","};
first = false;
str += QString::number(i);
}
return str;
}
case TransactionSubaddrAccountRole:
return tInfo.subaddrAccount();
case TransactionLabelRole:
return tInfo.subaddrIndex().size() == 1 && *tInfo.subaddrIndex().begin() == 0 ? tr("Primary address") : tInfo.label();
case TransactionConfirmationsRole:
return tInfo.confirmations();
case TransactionConfirmationsRequiredRole:
return (tInfo.blockHeight() < tInfo.unlockTime()) ? tInfo.unlockTime() - tInfo.blockHeight() : 10;
case TransactionHashRole:
return tInfo.hash();
case TransactionTimeStampRole:
return tInfo.timestamp();
case TransactionPaymentIdRole:
return tInfo.paymentId();
case TransactionIsOutRole:
return tInfo.direction() == TransactionInfo::Direction_Out;
case TransactionDateRole:
return tInfo.date();
case TransactionTimeRole:
return tInfo.time();
case TransactionDestinationsRole:
return tInfo.destinations_formatted();
default:
{
qCritical() << "Unimplemented role" << role;
return QVariant();
}
}
}
QVariant TransactionHistoryModel::data(const QModelIndex &index, int role) const QVariant TransactionHistoryModel::data(const QModelIndex &index, int role) const
{ {
if (!m_transactionHistory) { if (!m_transactionHistory) {
return QVariant(); return QVariant();
} }
if (index.row() < 0 || (unsigned)index.row() >= m_transactionHistory->count()) {
return QVariant();
}
TransactionInfo * tInfo = m_transactionHistory->transaction(index.row());
Q_ASSERT(tInfo);
if (!tInfo) {
qCritical("%s: internal error: no transaction info for index %d", __FUNCTION__, index.row());
return QVariant();
}
QVariant result; QVariant result;
switch (role) { bool found = m_transactionHistory->transaction(index.row(), [this, &result, &role](const TransactionInfo &tInfo) {
case TransactionRole: result = parseTransactionInfo(tInfo, role);
result = QVariant::fromValue(tInfo); });
break; if (!found) {
case TransactionDirectionRole: qCritical("%s: internal error: no transaction info for index %d", __FUNCTION__, index.row());
result = QVariant::fromValue(tInfo->direction());
break;
case TransactionPendingRole:
result = tInfo->isPending();
break;
case TransactionFailedRole:
result = tInfo->isFailed();
break;
case TransactionAmountRole:
result = tInfo->amount();
break;
case TransactionDisplayAmountRole:
result = tInfo->displayAmount();
break;
case TransactionAtomicAmountRole:
result = tInfo->atomicAmount();
break;
case TransactionFeeRole:
result = tInfo->fee();
break;
case TransactionBlockHeightRole:
// Use NULL QVariant for transactions without height.
// Forces them to be displayed at top when sorted by blockHeight.
if (tInfo->blockHeight() != 0) {
result = tInfo->blockHeight();
}
break;
case TransactionSubaddrIndexRole:
{
QString str = QString{""};
bool first = true;
for (quint32 i : tInfo->subaddrIndex()) {
if (!first)
str += QString{","};
first = false;
str += QString::number(i);
}
result = str;
}
break;
case TransactionSubaddrAccountRole:
result = tInfo->subaddrAccount();
break;
case TransactionLabelRole:
result = tInfo->subaddrIndex().size() == 1 && *tInfo->subaddrIndex().begin() == 0 ? tr("Primary address") : tInfo->label();
break;
case TransactionConfirmationsRole:
result = tInfo->confirmations();
break;
case TransactionConfirmationsRequiredRole:
result = (tInfo->blockHeight() < tInfo->unlockTime()) ? tInfo->unlockTime() - tInfo->blockHeight() : 10;
break;
case TransactionHashRole:
result = tInfo->hash();
break;
case TransactionTimeStampRole:
result = tInfo->timestamp();
break;
case TransactionPaymentIdRole:
result = tInfo->paymentId();
break;
case TransactionIsOutRole:
result = tInfo->direction() == TransactionInfo::Direction_Out;
break;
case TransactionDateRole:
result = tInfo->date();
break;
case TransactionTimeRole:
result = tInfo->time();
break;
case TransactionDestinationsRole:
result = tInfo->destinations_formatted();
break;
} }
return result; return result;
} }
@ -171,7 +155,6 @@ int TransactionHistoryModel::rowCount(const QModelIndex &parent) const
QHash<int, QByteArray> TransactionHistoryModel::roleNames() const QHash<int, QByteArray> TransactionHistoryModel::roleNames() const
{ {
QHash<int, QByteArray> roleNames = QAbstractListModel::roleNames(); QHash<int, QByteArray> roleNames = QAbstractListModel::roleNames();
roleNames.insert(TransactionRole, "transaction");
roleNames.insert(TransactionDirectionRole, "direction"); roleNames.insert(TransactionDirectionRole, "direction");
roleNames.insert(TransactionPendingRole, "isPending"); roleNames.insert(TransactionPendingRole, "isPending");
roleNames.insert(TransactionFailedRole, "isFailed"); roleNames.insert(TransactionFailedRole, "isFailed");

View file

@ -45,8 +45,7 @@ class TransactionHistoryModel : public QAbstractListModel
public: public:
enum TransactionInfoRole { enum TransactionInfoRole {
TransactionRole = Qt::UserRole + 1, // for the TransactionInfo object; TransactionDirectionRole = Qt::UserRole + 1,
TransactionDirectionRole,
TransactionPendingRole, TransactionPendingRole,
TransactionFailedRole, TransactionFailedRole,
TransactionAmountRole, TransactionAmountRole,
@ -97,6 +96,9 @@ public:
signals: signals:
void transactionHistoryChanged(); void transactionHistoryChanged();
private:
QVariant parseTransactionInfo(const TransactionInfo &tInfo, int role) const;
private: private:
TransactionHistory * m_transactionHistory; TransactionHistory * m_transactionHistory;
}; };