refactor(history): enforce using PK instead of ToxId in history

We need to be lenient when reading from db because of ToxIds being saved in the
db from a bug introduced in e07d8d358f, which
used self ID rather than self Pk. Was subsequently fixed in
033f28e67e.

src/core/toxid.cpp

@@ -114,7 +114,7 @@ ToxId::ToxId(const QByteArray& rawId)
  * If the given rawId isn't a valid Public Key or Tox ID a ToxId with all zero bytes is created.
  *
  * @param rawId Pointer to bytes to convert to ToxId object
- * @param len Number of bytes to read. Must be TOX_SECRET_KEY_SIZE for a Public Key or
+ * @param len Number of bytes to read. Must be TOX_PUBLIC_KEY_SIZE for a Public Key or
  *            TOX_ADDRESS_SIZE for a Tox ID.
  */
 ToxId::ToxId(const uint8_t* rawId, int len)
@@ -127,7 +127,7 @@ ToxId::ToxId(const uint8_t* rawId, int len)
 void ToxId::constructToxId(const QByteArray& rawId)
 {
     // TODO: remove construction from PK only
-    if (rawId.length() == TOX_SECRET_KEY_SIZE) {
+    if (rawId.length() == TOX_PUBLIC_KEY_SIZE) {
         toxId = QByteArray(rawId); // construct from PK only
     } else if (rawId.length() == TOX_ADDRESS_SIZE && isToxId(rawId.toHex().toUpper())) {
         toxId = QByteArray(rawId); // construct from full toxid

src/model/about/aboutfriend.cpp

@@ -123,7 +123,7 @@ bool AboutFriend::clearHistory()
     const ToxPk pk = f->getPublicKey();
     History* const history = Nexus::getProfile()->getHistory();
     if (history) {
-        history->removeFriendHistory(pk.toString());
+        history->removeFriendHistory(pk);
         return true;
     }
 

src/model/chathistory.cpp

@@ -155,7 +155,7 @@ SearchResult ChatHistory::searchBackward(SearchPos startIdx, const QString& phra
     // If the double disk access is real bad we can optimize this by adding
     // another function to history
     auto dateWherePhraseFound =
-        history->getDateWhereFindPhrase(f.getPublicKey().toString(), earliestMessageDate, phrase,
+        history->getDateWhereFindPhrase(f.getPublicKey(), earliestMessageDate, phrase,
                                         parameter);
 
     if (dateWherePhraseFound.isValid()) {
@@ -219,8 +219,8 @@ void ChatHistory::onFileUpdated(const ToxPk& sender, const ToxFile& file)
             // chat log. Both rely on generating a new id based on the state of
             // initializing. If this is changed in the session chat log we'll end up
             // with a different order when loading from history
-            history->addNewFileMessage(f.getPublicKey().toString(), file.resumeFileId, file.fileName,
+            history->addNewFileMessage(f.getPublicKey(), file.resumeFileId, file.fileName,
-                                       file.filePath, file.filesize, sender.toString(),
+                                       file.filePath, file.filesize, sender,
                                        QDateTime::currentDateTime(), f.getDisplayedName());
             break;
         }
@@ -256,7 +256,7 @@ void ChatHistory::onFileTransferBrokenUnbroken(const ToxPk& sender, const ToxFil
 void ChatHistory::onMessageReceived(const ToxPk& sender, const Message& message)
 {
     if (canUseHistory()) {
-        auto friendPk = f.getPublicKey().toString();
+        auto friendPk = f.getPublicKey();
         auto displayName = f.getDisplayedName();
         auto content = message.content;
         if (message.isAction) {
@@ -272,8 +272,8 @@ void ChatHistory::onMessageReceived(const ToxPk& sender, const Message& message)
 void ChatHistory::onMessageSent(DispatchedMessageId id, const Message& message)
 {
     if (canUseHistory()) {
-        auto selfPk = coreIdHandler.getSelfPublicKey().toString();
+        auto selfPk = coreIdHandler.getSelfPublicKey();
-        auto friendPk = f.getPublicKey().toString();
+        auto friendPk = f.getPublicKey();
 
         auto content = message.content;
         if (message.isAction) {

src/persistence/history.cpp

@@ -24,6 +24,7 @@
 #include "profile.h"
 #include "settings.h"
 #include "db/rawdatabase.h"
+#include "src/core/toxpk.h"
 
 namespace {
 static constexpr int SCHEMA_VERSION = 4;
@@ -332,7 +333,10 @@ History::History(std::shared_ptr<RawDatabase> db_)
     // Cache our current peers
     db->execLater(RawDatabase::Query{"SELECT public_key, id FROM peers;",
                                      [this](const QVector<QVariant>& row) {
-                                         peers[row[0].toString()] = row[1].toInt();
+                                         // HACK: we previously accidentally put Tox IDs in the db. So instead of
+                                         // constructing as a ToxPk which will enforce the correct length, construct
+                                         // as ToxId which will allow either length, and then convert to ToxPk.
+                                         peers[ToxId{QByteArray::fromHex(row[0].toByteArray())}.getPublicKey()] = row[1].toInt();
                                      }});
 }
 
@@ -392,7 +396,7 @@ void History::eraseHistory()
  * @brief Erases the chat history with one friend.
  * @param friendPk Friend public key to erase.
  */
-void History::removeFriendHistory(const QString& friendPk)
+void History::removeFriendHistory(const ToxPk& friendPk)
 {
     if (!isValid()) {
         return;
@@ -441,8 +445,8 @@ void History::removeFriendHistory(const QString& friendPk)
  * @param insertIdCallback Function, called after query execution.
  */
 QVector<RawDatabase::Query>
-History::generateNewMessageQueries(const QString& friendPk, const QString& message,
+History::generateNewMessageQueries(const ToxPk& friendPk, const QString& message,
-                                   const QString& sender, const QDateTime& time, bool isDelivered,
+                                   const ToxPk& sender, const QDateTime& time, bool isDelivered,
                                    QString dispName, std::function<void(RowId)> insertIdCallback)
 {
     QVector<RawDatabase::Query> queries;
@@ -461,7 +465,7 @@ History::generateNewMessageQueries(const QString& friendPk, const QString& messa
         (peers)[friendPk] = peerId;
         queries += RawDatabase::Query(("INSERT INTO peers (id, public_key) "
                                        "VALUES (%1, '"
-                                       + friendPk + "');")
+                                       + friendPk.toString() + "');")
                                           .arg(peerId));
     }
 
@@ -479,7 +483,7 @@ History::generateNewMessageQueries(const QString& friendPk, const QString& messa
         (peers)[sender] = senderId;
         queries += RawDatabase::Query{("INSERT INTO peers (id, public_key) "
                                        "VALUES (%1, '"
-                                       + sender + "');")
+                                       + sender.toString() + "');")
                                           .arg(senderId)};
     }
 
@@ -581,9 +585,9 @@ RawDatabase::Query History::generateFileFinished(RowId id, bool success, const Q
     }
 }
 
-void History::addNewFileMessage(const QString& friendPk, const QString& fileId,
+void History::addNewFileMessage(const ToxPk& friendPk, const QString& fileId,
                                 const QString& fileName, const QString& filePath, int64_t size,
-                                const QString& sender, const QDateTime& time, QString const& dispName)
+                                const ToxPk& sender, const QDateTime& time, QString const& dispName)
 {
     if (historyAccessBlocked()) {
         return;
@@ -641,7 +645,7 @@ void History::addNewFileMessage(const QString& friendPk, const QString& fileId,
  * @param dispName Name, which should be displayed.
  * @param insertIdCallback Function, called after query execution.
  */
-void History::addNewMessage(const QString& friendPk, const QString& message, const QString& sender,
+void History::addNewMessage(const ToxPk& friendPk, const QString& message, const ToxPk& sender,
                             const QDateTime& time, bool isDelivered, QString dispName,
                             const std::function<void(RowId)>& insertIdCallback)
 {
@@ -823,7 +827,7 @@ QList<History::HistMessage> History::getUndeliveredMessagesForFriend(const ToxPk
  * @param parameter for search
  * @return date of the message where the phrase was found
  */
-QDateTime History::getDateWhereFindPhrase(const QString& friendPk, const QDateTime& from,
+QDateTime History::getDateWhereFindPhrase(const ToxPk& friendPk, const QDateTime& from,
                                           QString phrase, const ParameterSearch& parameter)
 {
     if (historyAccessBlocked()) {
@@ -899,7 +903,7 @@ QDateTime History::getDateWhereFindPhrase(const QString& friendPk, const QDateTi
                        "WHERE chat.public_key='%1' "
                        "AND %2 "
                        "%3")
-            .arg(friendPk)
+            .arg(friendPk.toString())
             .arg(message)
             .arg(period);
 

src/persistence/history.h

@@ -95,7 +95,7 @@ struct FileDbInsertionData
     FileDbInsertionData();
 
     RowId historyId;
-    QString friendPk;
+    ToxPk friendPk;
     QString fileId;
     QString fileName;
     QString filePath;
@@ -164,21 +164,21 @@ public:
     bool historyExists(const ToxPk& friendPk);
 
     void eraseHistory();
-    void removeFriendHistory(const QString& friendPk);
+    void removeFriendHistory(const ToxPk& friendPk);
-    void addNewMessage(const QString& friendPk, const QString& message, const QString& sender,
+    void addNewMessage(const ToxPk& friendPk, const QString& message, const ToxPk& sender,
                        const QDateTime& time, bool isDelivered, QString dispName,
                        const std::function<void(RowId)>& insertIdCallback = {});
 
-    void addNewFileMessage(const QString& friendPk, const QString& fileId,
+    void addNewFileMessage(const ToxPk& friendPk, const QString& fileId,
                            const QString& fileName, const QString& filePath, int64_t size,
-                           const QString& sender, const QDateTime& time, QString const& dispName);
+                           const ToxPk& sender, const QDateTime& time, QString const& dispName);
 
     void setFileFinished(const QString& fileId, bool success, const QString& filePath, const QByteArray& fileHash);
     size_t getNumMessagesForFriend(const ToxPk& friendPk);
     size_t getNumMessagesForFriendBeforeDate(const ToxPk& friendPk, const QDateTime& date);
     QList<HistMessage> getMessagesForFriend(const ToxPk& friendPk, size_t firstIdx, size_t lastIdx);
     QList<HistMessage> getUndeliveredMessagesForFriend(const ToxPk& friendPk);
-    QDateTime getDateWhereFindPhrase(const QString& friendPk, const QDateTime& from, QString phrase,
+    QDateTime getDateWhereFindPhrase(const ToxPk& friendPk, const QDateTime& from, QString phrase,
                                      const ParameterSearch& parameter);
     QList<DateIdx> getNumMessagesForFriendBeforeDateBoundaries(const ToxPk& friendPk,
                                                                const QDate& from, size_t maxNum);
@@ -187,8 +187,8 @@ public:
 
 protected:
     QVector<RawDatabase::Query>
-    generateNewMessageQueries(const QString& friendPk, const QString& message,
+    generateNewMessageQueries(const ToxPk& friendPk, const QString& message,
-                              const QString& sender, const QDateTime& time, bool isDelivered,
+                              const ToxPk& sender, const QDateTime& time, bool isDelivered,
                               QString dispName, std::function<void(RowId)> insertIdCallback = {});
 
 signals:
@@ -206,7 +206,7 @@ private:
     std::shared_ptr<RawDatabase> db;
 
 
-    QHash<QString, int64_t> peers;
+    QHash<ToxPk, int64_t> peers;
     struct FileInfo
     {
         bool finished = false;

src/persistence/profile.cpp

@@ -687,12 +687,11 @@ void Profile::onRequestSent(const ToxPk& friendPk, const QString& message)
         return;
     }
 
-    const QString pkStr = friendPk.toString();
     const QString inviteStr = Core::tr("/me offers friendship, \"%1\"").arg(message);
-    const QString selfStr = core->getSelfPublicKey().toString();
+    const ToxPk selfPk = core->getSelfPublicKey();
     const QDateTime datetime = QDateTime::currentDateTime();
     const QString name = core->getUsername();
-    history->addNewMessage(pkStr, inviteStr, selfStr, datetime, true, name);
+    history->addNewMessage(friendPk, inviteStr, selfPk, datetime, true, name);
 }
 
 /**

src/widget/widget.cpp

@@ -1713,7 +1713,7 @@ void Widget::removeFriend(Friend* f, bool fake)
         }
 
         if (ask.removeHistory()) {
-            Nexus::getProfile()->getHistory()->removeFriendHistory(f->getPublicKey().toString());
+            Nexus::getProfile()->getHistory()->removeFriendHistory(f->getPublicKey());
         }
     }
 

test/core/contactid_test.cpp

@@ -50,6 +50,7 @@ private slots:
     void copyTest();
     void dataTest();
     void sizeTest();
+    void hashableTest();
 };
 
 void TestContactId::toStringTest()
@@ -100,5 +101,14 @@ void TestContactId::sizeTest()
     QVERIFY(id.getSize() == TOX_CONFERENCE_UID_SIZE);
 }
 
+void TestContactId::hashableTest()
+{
+    ToxPk pk1{testPkArray};
+    ToxPk pk2{testPk};
+    QVERIFY(qHash(pk1) == qHash(pk2));
+    ToxPk pk3{echoPk};
+    QVERIFY(qHash(pk1) != qHash(pk3));
+}
+
 QTEST_GUILESS_MAIN(TestContactId)
 #include "contactid_test.moc"