Address catenocrypt's review

Author: riptl

src/Filecoin/Address.cpp

@@ -19,23 +19,26 @@ bool Address::isValid(const Data& data) {
     if (data.size() < 2) {
         return false;
     }
-    if (!getType(data[0])) {
+    Type type = getType(data[0]);
+    if (type == Type::Invalid) {
         return false;
-    }
-    Type type = *getType(data[0]);
-    if (type == Type::ID) {
+    } else if (type == Type::ID) {
         // Verify varuint encoding
-        if (data.size() > 11)
+        if (data.size() > 11) {
             return false;
-        if (data.size() == 11 && data[10] > 0x01)
+        }
+        if (data.size() == 11 && data[10] > 0x01) {
             return false;
+        }
         int i;
-        for (i = 1; i < data.size(); i++)
-            if ((data[i] & 0x80) == 0)
+        for (i = 1; i < data.size(); i++) {
+            if ((data[i] & 0x80) == 0) {
                 break;
+            }
+        }
         return i == data.size() - 1;
     } else {
-        return data.size() == 1 + Address::payloadSize(type);
+        return data.size() == (1 + Address::payloadSize(type));
     }
 }
 
@@ -49,7 +52,7 @@ static bool isValidID(const std::string& string) {
     }
     try {
         size_t chars;
-        std::stoull(string.substr(2), &chars);
+        [[maybe_unused]] uint64_t id = std::stoull(string.substr(2), &chars);
         return chars > 0;
     } catch (...) {
         return false;
@@ -65,7 +68,7 @@ static bool isValidBase32(const std::string& string, Address::Type type) {
     }
 
     // Check size
-    if (decoded.size() != size + 4) {
+    if (decoded.size() != size + checksumSize) {
         return false;
     }
 
@@ -89,19 +92,19 @@ bool Address::isValid(const std::string& string) {
     }
     // Get address type.
     auto type = parseType(string[1]);
-    if (!type) {
+    if (type == Type::Invalid) {
         return false;
     }
 
     // ID addresses are special, they are just numbers.
-    return type == Type::ID ? isValidID(string) : isValidBase32(string, *type);
+    return type == Type::ID ? isValidID(string) : isValidBase32(string, type);
 }
 
 Address::Address(const std::string& string) {
     if (!isValid(string))
         throw std::invalid_argument("Invalid address data");
 
-    Type type = *(parseType(string[1]));
+    Type type = parseType(string[1]);
     // First byte is type
     bytes.push_back(static_cast<uint8_t>(type));
     if (type == Type::ID) {

src/Filecoin/Address.h

@@ -10,7 +10,6 @@
 
 #include <array>
 #include <cstdint>
-#include <optional>
 #include <string>
 #include <vector>
 
@@ -23,6 +22,7 @@ class Address {
         SECP256K1 = 1,
         ACTOR = 2,
         BLS = 3,
+        Invalid,
     };
 
     /// Address data with address type prefix.
@@ -47,14 +47,14 @@ class Address {
     [[nodiscard]] std::string string() const;
 
     /// Returns the type of an address.
-    Type type() const { return *getType(bytes[0]); }
+    Type type() const { return getType(bytes[0]); }
 
     /// Address prefix
     static constexpr char PREFIX = 'f';
 
   public:
     /// Attempts to get the type by number.
-    static std::optional<Type> getType(uint8_t raw) {
+    static Type getType(uint8_t raw) {
         switch (raw) {
         case 0:
             return Type::ID;
@@ -65,16 +65,16 @@ class Address {
         case 3:
             return Type::BLS;
         default:
-            return {};
+            return Type::Invalid;
         }
     }
 
     /// Attempts to get the type by ASCII.
-    static std::optional<Type> parseType(char c) {
+    static Type parseType(char c) {
         if (c >= '0' && c <= '3') {
             return static_cast<Type>(c - '0');
         } else {
-            return {};
+            return Type::Invalid;
         }
     }
 

src/Filecoin/Transaction.cpp

@@ -14,9 +14,11 @@ static Data encodeVaruint(const uint256_t& value) {
     Data data;
     encode256BE(data, value, 256);
     size_t i = 0;
-    for (i = 0; i < data.size(); ++i)
-        if (data[i] != 0)
+    for (i = 0; i < data.size(); ++i) {
+        if (data[i] != 0) {
             break;
+        }
+    }
     Data small;
     small.assign(data.begin() + i - 1, data.end());
     return small;
@@ -54,8 +56,8 @@ Data Transaction::cid() const {
 Data Transaction::serialize(Data& signature) const {
     // Wrap signature in object.
     Data sigObject;
-    sigObject.push_back(0x01); // Prepend IKTSecp256k1 type
-    sigObject.insert(sigObject.end(), signature.begin(), signature.end());
+    TW::append(sigObject, 0x01); // Prepend IKTSecp256k1 type
+    TW::append(sigObject, signature);
     // Create Filecoin SignedMessage
     auto signedMessage = Cbor::Encode::array({message(), Cbor::Encode::bytes(sigObject)});
     return signedMessage.encoded();

tests/Filecoin/TransactionTests.cpp

@@ -1,4 +1,4 @@
-// Copyright © 2017-2019 Trust.
+// Copyright © 2017-2020 Trust.
 //
 // This file is part of Trust. The full Trust copyright notice, including
 // terms governing use, modification, and redistribution, is contained in the