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.

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 PeerTableModel::StatsRole to prevent data layer violation #161

Merged
merged 3 commits into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions src/qt/peertablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const
default:
return QVariant();
}
} else if (role == StatsRole) {
switch (index.column()) {
case NetNodeId: return QVariant::fromValue(rec);
default: return QVariant();
}
}

return QVariant();
Expand Down Expand Up @@ -216,11 +221,6 @@ QModelIndex PeerTableModel::index(int row, int column, const QModelIndex &parent
return QModelIndex();
}

const CNodeCombinedStats *PeerTableModel::getNodeStats(int idx)
{
return priv->index(idx);
}

void PeerTableModel::refresh()
{
Q_EMIT layoutAboutToBeChanged();
Expand Down
6 changes: 5 additions & 1 deletion src/qt/peertablemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ struct CNodeCombinedStats {
CNodeStateStats nodeStateStats;
bool fNodeStateStatsAvailable;
};
Q_DECLARE_METATYPE(CNodeCombinedStats*)

class NodeLessThan
{
Expand All @@ -52,7 +53,6 @@ class PeerTableModel : public QAbstractTableModel
public:
explicit PeerTableModel(interfaces::Node& node, QObject* parent);
~PeerTableModel();
const CNodeCombinedStats *getNodeStats(int idx);
int getRowByNodeId(NodeId nodeid);
void startAutoRefresh();
void stopAutoRefresh();
Expand All @@ -67,6 +67,10 @@ class PeerTableModel : public QAbstractTableModel
Subversion = 6
};

enum {
StatsRole = Qt::UserRole,
};
hebasto marked this conversation as resolved.
Show resolved Hide resolved

/** @name Methods overridden from QAbstractTableModel
@{*/
int rowCount(const QModelIndex &parent) const override;
Expand Down
28 changes: 7 additions & 21 deletions src/qt/rpcconsole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1018,11 +1018,9 @@ void RPCConsole::updateTrafficStats(quint64 totalBytesIn, quint64 totalBytesOut)

void RPCConsole::peerLayoutAboutToChange()
{
QModelIndexList selected = ui->peerWidget->selectionModel()->selectedIndexes();
cachedNodeids.clear();
for(int i = 0; i < selected.size(); i++)
{
const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(selected.at(i).row());
for (const QModelIndex& peer : GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId)) {
const auto stats = peer.data(PeerTableModel::StatsRole).value<CNodeCombinedStats*>();
hebasto marked this conversation as resolved.
Show resolved Hide resolved
cachedNodeids.append(stats->nodeStats.nodeid);
}
}
Expand Down Expand Up @@ -1081,15 +1079,13 @@ void RPCConsole::peerLayoutChanged()

void RPCConsole::updateDetailWidget()
{
QModelIndexList selected_rows;
auto selection_model = ui->peerWidget->selectionModel();
if (selection_model) selected_rows = selection_model->selectedRows();
if (!clientModel || !clientModel->getPeerTableModel() || selected_rows.size() != 1) {
const QList<QModelIndex> selected_peers = GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId);
if (!clientModel || !clientModel->getPeerTableModel() || selected_peers.size() != 1) {
ui->detailWidget->hide();
ui->peerHeading->setText(tr("Select a peer to view detailed information."));
return;
}
const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(selected_rows.first().row());
const auto stats = selected_peers.first().data(PeerTableModel::StatsRole).value<CNodeCombinedStats*>();
hebasto marked this conversation as resolved.
Show resolved Hide resolved
// update the detail ui with latest node information
QString peerAddrDetails(QString::fromStdString(stats->nodeStats.addrName) + " ");
peerAddrDetails += tr("(peer id: %1)").arg(QString::number(stats->nodeStats.nodeid));
Expand Down Expand Up @@ -1202,19 +1198,9 @@ void RPCConsole::banSelectedNode(int bantime)
if (!clientModel)
return;

// Get selected peer addresses
QList<QModelIndex> nodes = GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId);
for(int i = 0; i < nodes.count(); i++)
{
// Get currently selected peer address
NodeId id = nodes.at(i).data().toLongLong();

// Get currently selected peer address
int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(id);
if (detailNodeRow < 0) return;

for (const QModelIndex& peer : GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId)) {
// Find possible nodes, ban it and clear the selected node
const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
const auto stats = peer.data(PeerTableModel::StatsRole).value<CNodeCombinedStats*>();
hebasto marked this conversation as resolved.
Show resolved Hide resolved
if (stats) {
m_node.ban(stats->nodeStats.addr, bantime);
m_node.disconnectByAddress(stats->nodeStats.addr);
Expand Down