refactor(ToxPk): Remove ability to construct ToxId from ToxPk

Construct ToxPk directly everywhere that used to construct through ToxId.

Now any ToxId should be a valid ToxId, and not possibly just a ToxPk.

src/chatlog/chatwidget.cpp

@@ -1404,7 +1404,7 @@ void ChatWidget::renderItem(const ChatLogItem& item, bool hideName, bool coloriz
 {
     const auto& sender = item.getSender();
 
-    bool isSelf = sender == core.getSelfId().getPublicKey();
+    bool isSelf = sender == core.getSelfPublicKey();
 
     switch (item.getContentType()) {
     case ChatLogItem::ContentType::message: {

src/core/core.cpp

@@ -838,8 +838,7 @@ void Core::bootstrapDht()
             address = dhtServer.ipv4.toLatin1();
         }
 
-        // TODO: constucting the pk via ToxId is a workaround
-        ToxPk pk = ToxId{dhtServer.userId}.getPublicKey();
+        ToxPk pk{dhtServer.userId};
         const uint8_t* pkPtr = pk.getData();
 
         Tox_Err_Bootstrap error;

src/core/toxid.cpp

@@ -18,19 +18,21 @@
 */
 
 
+#include "contactid.h"
 #include "toxid.h"
 #include "toxpk.h"
 
 #include <tox/tox.h>
 
 #include <QRegularExpression>
+
+#include <cassert>
 #include <cstdint>
 
 // Tox doesn't publicly define these
 #define NOSPAM_BYTES 4
 #define CHECKSUM_BYTES 2
 
-#define PUBLIC_KEY_HEX_CHARS (2 * TOX_PUBLIC_KEY_SIZE)
 #define NOSPAM_HEX_CHARS (2 * NOSPAM_BYTES)
 #define CHECKSUM_HEX_CHARS (2 * CHECKSUM_BYTES)
 #define TOXID_HEX_CHARS (2 * TOX_ADDRESS_SIZE)
@@ -59,8 +61,7 @@ const QRegularExpression ToxId::ToxIdRegEx(QString("(^|\\s)[A-Fa-f0-9]{%1}($|\\s
  */
 ToxId::ToxId()
     : toxId()
-{
-}
+{}
 
 /**
  * @brief The copy constructor.
@@ -68,8 +69,7 @@ ToxId::ToxId()
  */
 ToxId::ToxId(const ToxId& other)
     : toxId(other.toxId)
-{
-}
+{}
 
 /**
  * @brief Create a Tox ID from a QString.
@@ -82,12 +82,10 @@ ToxId::ToxId(const ToxId& other)
  */
 ToxId::ToxId(const QString& id)
 {
-    // TODO: remove construction from PK only
     if (isToxId(id)) {
         toxId = QByteArray::fromHex(id.toLatin1());
-    } else if (id.length() >= PUBLIC_KEY_HEX_CHARS) {
-        toxId = QByteArray::fromHex(id.left(PUBLIC_KEY_HEX_CHARS).toLatin1());
     } else {
+        assert(!"ToxId constructed with invalid length string");
         toxId = QByteArray(); // invalid id string
     }
 }
@@ -126,12 +124,10 @@ ToxId::ToxId(const uint8_t* rawId, int len)
 
 void ToxId::constructToxId(const QByteArray& rawId)
 {
-    // TODO: remove construction from PK only
-    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())) {
+    if (rawId.length() == TOX_ADDRESS_SIZE && isToxId(rawId.toHex().toUpper())) {
         toxId = QByteArray(rawId); // construct from full toxid
     } else {
+        assert(!"ToxId constructed with invalid input");
         toxId = QByteArray(); // invalid id
     }
 }

src/core/toxpk.cpp

@@ -17,6 +17,7 @@
     along with qTox.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include "contactid.h"
 #include "toxpk.h"
 
 #include <tox/tox.h>
@@ -26,6 +27,8 @@
 
 #include <cassert>
 
+#define PUBLIC_KEY_HEX_CHARS (2 * TOX_PUBLIC_KEY_SIZE)
+
 /**
  * @class ToxPk
  * @brief This class represents a Tox Public Key, which is a part of Tox ID.
@@ -54,7 +57,7 @@ ToxPk::ToxPk(const ToxPk& other)
  *              TOX_PUBLIC_KEY_SIZE, else the ToxPk will be empty.
  */
 ToxPk::ToxPk(const QByteArray& rawId)
-    : ContactId([rawId](){
+    : ContactId([&rawId](){
         assert(rawId.length() == TOX_PUBLIC_KEY_SIZE);
         return rawId;}())
 {
@@ -70,6 +73,25 @@ ToxPk::ToxPk(const uint8_t* rawId)
 {
 }
 
+/**
+ * @brief Constructs a ToxPk from a QString.
+ *
+  * If the given pk isn't a valid Public Key a ToxPk with all zero bytes is created.
+ *
+ * @param pk Tox Pk string to convert to ToxPk object
+ */
+ToxPk::ToxPk(const QString& pk)
+    : ContactId([&pk](){
+    if (pk.length() == PUBLIC_KEY_HEX_CHARS) {
+        return QByteArray::fromHex(pk.toLatin1());
+    } else {
+        assert(!"ToxPk constructed with invalid length string");
+        return QByteArray(); // invalid pk string
+    }
+    }())
+{
+}
+
 /**
  * @brief Get size of public key in bytes.
  * @return Size of public key in bytes.

src/core/toxpk.h

@@ -30,5 +30,6 @@ public:
     ToxPk(const ToxPk& other);
     explicit ToxPk(const QByteArray& rawId);
     explicit ToxPk(const uint8_t* rawId);
+    explicit ToxPk(const QString& pk);
     int getSize() const override;
 };

src/model/chathistory.cpp

@@ -361,7 +361,7 @@ void ChatHistory::loadHistoryIntoSessionChatLog(ChatLogIdx start) const
         // Note that message.id is _not_ a valid conversion here since it is a
         // global id not a per-chat id like the ChatLogIdx
         auto currentIdx = nextIdx++;
-        auto sender = ToxId(message.sender).getPublicKey();
+        auto sender = ToxPk(message.sender);
         switch (message.content.getType()) {
         case HistMessageContentType::file: {
             const auto date = message.timestamp;

src/model/friend.h

@@ -22,7 +22,7 @@
 #include "contact.h"
 #include "src/core/core.h"
 #include "src/core/extension.h"
-#include "src/core/toxid.h"
+#include "src/core/toxpk.h"
 #include "src/core/contactid.h"
 #include "src/model/status.h"
 #include <QObject>

src/model/group.cpp

@@ -181,7 +181,7 @@ bool Group::getMentionedFlag() const
     return userWasMentioned;
 }
 
-QString Group::resolveToxId(const ToxPk& id) const
+QString Group::resolveToxPk(const ToxPk& id) const
 {
     auto it = peerDisplayNames.find(id);
 

src/model/group.h

@@ -56,7 +56,7 @@ public:
     void setTitle(const QString& author, const QString& newTitle);
     QString getName() const;
     QString getDisplayedName() const override;
-    QString resolveToxId(const ToxPk& id) const;
+    QString resolveToxPk(const ToxPk& id) const;
     void setSelfName(const QString& name);
     QString getSelfName() const;
 

src/model/sessionchatlog.cpp

@@ -115,7 +115,7 @@ QString resolveToxPk(const ToxPk& pk)
     }
 
     for (Group* it : GroupList::getAllGroups()) {
-        QString res = it->resolveToxId(pk);
+        QString res = it->resolveToxPk(pk);
         if (!res.isEmpty()) {
             return res;
         }

src/persistence/profile.cpp

@@ -537,7 +537,7 @@ QString Profile::avatarPath(const ToxPk& owner, bool forceUnencrypted)
     }
 
     QByteArray idData = ownerStr.toUtf8();
-    QByteArray pubkeyData = core->getSelfId().getPublicKey().getByteArray();
+    QByteArray pubkeyData = core->getSelfPublicKey().getByteArray();
     constexpr int hashSize = TOX_PUBLIC_KEY_SIZE;
     static_assert(hashSize >= crypto_generichash_BYTES_MIN && hashSize <= crypto_generichash_BYTES_MAX,
                   "Hash size not supported by libsodium");
@@ -556,7 +556,7 @@ QString Profile::avatarPath(const ToxPk& owner, bool forceUnencrypted)
  */
 QPixmap Profile::loadAvatar()
 {
-    return loadAvatar(core->getSelfId().getPublicKey());
+    return loadAvatar(core->getSelfPublicKey());
 }
 
 /**
@@ -623,7 +623,7 @@ void Profile::loadDatabase(QString password)
         return;
     }
 
-    QByteArray salt = core->getSelfId().getPublicKey().getByteArray();
+    QByteArray salt = core->getSelfPublicKey().getByteArray();
     if (salt.size() != TOX_PASS_SALT_LENGTH) {
         qWarning() << "Couldn't compute salt from public key" << name;
         GUI::showError(QObject::tr("Error"),
@@ -758,7 +758,7 @@ QByteArray Profile::getAvatarHash(const ToxPk& owner)
  */
 void Profile::removeSelfAvatar()
 {
-    removeAvatar(core->getSelfId().getPublicKey());
+    removeAvatar(core->getSelfPublicKey());
 }
 
 /**
@@ -795,7 +795,7 @@ History* Profile::getHistory()
 void Profile::removeAvatar(const ToxPk& owner)
 {
     QFile::remove(avatarPath(owner));
-    if (owner == core->getSelfId().getPublicKey()) {
+    if (owner == core->getSelfPublicKey()) {
         setAvatar({});
     } else {
         setFriendAvatar(owner, {});
@@ -966,8 +966,8 @@ QString Profile::setPassword(const QString& newPassword)
                    "password.");
     }
 
-    QByteArray avatar = loadAvatarData(core->getSelfId().getPublicKey());
-    saveAvatar(core->getSelfId().getPublicKey(), avatar);
+    QByteArray avatar = loadAvatarData(core->getSelfPublicKey());
+    saveAvatar(core->getSelfPublicKey(), avatar);
 
     QVector<uint32_t> friendList = core->getFriendList();
     QVectorIterator<uint32_t> i(friendList);

src/persistence/profile.h

@@ -22,7 +22,6 @@
 
 #include "src/core/core.h"
 #include "src/core/toxencrypt.h"
-#include "src/core/toxid.h"
 
 #include "src/net/avatarbroadcaster.h"
 
@@ -38,6 +37,7 @@
 
 class Settings;
 class QCommandLineParser;
+class ToxPk;
 
 class Profile : public QObject
 {

src/persistence/settings.cpp

@@ -504,7 +504,7 @@ void Settings::loadPersonal(QString profileName, const ToxEncrypt* passKey)
 
             if (getEnableLogging())
                 fp.activity = ps.value("activity", QDateTime()).toDateTime();
-            friendLst.insert(ToxId(fp.addr).getPublicKey().getByteArray(), fp);
+            friendLst.insert(ToxPk(fp.addr).getByteArray(), fp);
         }
         ps.endArray();
     }
@@ -1843,8 +1843,7 @@ void Settings::setCamVideoFPS(float newValue)
 QString Settings::getFriendAddress(const QString& publicKey) const
 {
     QMutexLocker locker{&bigLock};
-    // TODO: using ToxId here is a hack
-    QByteArray key = ToxId(publicKey).getPublicKey().getByteArray();
+    QByteArray key = ToxPk(publicKey).getByteArray();
     auto it = friendLst.find(key);
     if (it != friendLst.end())
         return it->addr;
@@ -1855,8 +1854,7 @@ QString Settings::getFriendAddress(const QString& publicKey) const
 void Settings::updateFriendAddress(const QString& newAddr)
 {
     QMutexLocker locker{&bigLock};
-    // TODO: using ToxId here is a hack
-    auto key = ToxId(newAddr).getPublicKey();
+    auto key = ToxPk(newAddr);
     auto& frnd = getOrInsertFriendPropRef(key);
     frnd.addr = newAddr;
 }

src/widget/form/genericchatform.cpp

@@ -101,7 +101,7 @@ QString GenericChatForm::resolveToxPk(const ToxPk& pk)
     }
 
     for (Group* it : GroupList::getAllGroups()) {
-        QString res = it->resolveToxId(pk);
+        QString res = it->resolveToxPk(pk);
         if (!res.isEmpty()) {
             return res;
         }