Added unit tests, and fixed two bugs: (#53)

* Integer conversion checks in src/wire/read.h
 * Missing "boolean" function in wire::writer and derived types

src/rpc/light_wallet.cpp

@@ -165,7 +165,7 @@ namespace lws
 
   void rpc::read_bytes(wire::json_reader& source, safe_uint64& self)
   {
-    self = safe_uint64(wire::integer::convert_to<std::uint64_t>(source.safe_unsigned_integer()));
+    self = safe_uint64(wire::integer::cast_unsigned<std::uint64_t>(source.safe_unsigned_integer()));
   }
   void rpc::write_bytes(wire::json_writer& dest, const safe_uint64 self)
   {
@@ -175,7 +175,7 @@ namespace lws
   void rpc::read_bytes(wire::json_reader& source, safe_uint64_array& self)
   {
     for (std::size_t count = source.start_array(); !source.is_array_end(count); --count)
-      self.values.emplace_back(wire::integer::convert_to<std::uint64_t>(source.safe_unsigned_integer()));
+      self.values.emplace_back(wire::integer::cast_unsigned<std::uint64_t>(source.safe_unsigned_integer()));
     source.end_array();
   }
 

src/wire/json/read.cpp

@@ -233,13 +233,22 @@ namespace wire
     return json_bool.value.boolean;
   }
 
+  using imax_limits = std::numeric_limits<std::intmax_t>;
+  static_assert(0 <= imax_limits::max(), "expected 0 <= intmax_t::max");
+  static_assert(
+    imax_limits::max() <= std::numeric_limits<std::uintmax_t>::max(),
+    "expected intmax_t::max <= uintmax_t::max"
+  );
+
   std::intmax_t json_reader::integer()
   {
     rapidjson_sax json_int{error::schema::integer};
     read_next_value(json_int);
     if (json_int.negative)
       return json_int.value.integer;
-    return integer::convert_to<std::intmax_t>(json_int.value.unsigned_integer);
+    if (static_cast<std::uintmax_t>(imax_limits::max()) < json_int.value.unsigned_integer)
+      WIRE_DLOG_THROW_(error::schema::smaller_integer);
+    return static_cast<std::intmax_t>(json_int.value.unsigned_integer);
   }
 
   std::uintmax_t json_reader::unsigned_integer()
@@ -248,7 +257,9 @@ namespace wire
     read_next_value(json_uint);
     if (!json_uint.negative)
       return json_uint.value.unsigned_integer;
-    return integer::convert_to<std::uintmax_t>(json_uint.value.integer);
+    if (json_uint.value.integer < 0)
+      WIRE_DLOG_THROW_(error::schema::larger_integer);
+    return static_cast<std::uintmax_t>(json_uint.value.integer);
   }
     /*
   const std::vector<std::uintmax_t>& json_reader::unsigned_integer_array()

src/wire/json/write.cpp

@@ -73,6 +73,12 @@ namespace wire
     return buf;
   }
 
+  void json_writer::boolean(const bool source)
+  {
+    formatter_.Bool(source);
+    check_flush();
+  }
+
   void json_writer::integer(const int source)
   {
     formatter_.Int(source);

src/wire/json/write.h

@@ -85,6 +85,8 @@ namespace wire
     //! \return Null-terminated buffer containing uint as decimal ascii
     static std::array<char, uint_to_string_size> to_string(std::uintmax_t) noexcept;
 
+    void boolean(bool) override final;
+
     void integer(int) override final;
     void integer(std::intmax_t) override final;
 

src/wire/read.cpp

@@ -35,9 +35,16 @@ void wire::reader::increment_depth()
     WIRE_DLOG_THROW_(error::schema::maximum_depth);
 }
 
-[[noreturn]] void wire::integer::throw_exception(std::intmax_t source, std::intmax_t min)
+[[noreturn]] void wire::integer::throw_exception(std::intmax_t source, std::intmax_t min, std::intmax_t max)
 {
-  WIRE_DLOG_THROW(error::schema::larger_integer, source << " given when " << min << " is minimum permitted");
+  static_assert(
+    std::numeric_limits<std::intmax_t>::max() <= std::numeric_limits<std::uintmax_t>::max(),
+    "expected intmax_t::max <= uintmax_t::max"
+  );
+  if (source < 0)
+    WIRE_DLOG_THROW(error::schema::larger_integer, source << " given when " << min << " is minimum permitted");
+  else
+    throw_exception(std::uintmax_t(source), std::uintmax_t(max));
 }
 [[noreturn]] void wire::integer::throw_exception(std::uintmax_t source, std::uintmax_t max)
 {

src/wire/read.h

@@ -83,7 +83,7 @@ namespace wire
     //! \throw wire::exception if next value not a boolean.
     virtual bool boolean() = 0;
 
-    //! \throw wire::expception if next value not an integer.
+    //! \throw wire::exception if next value not an integer.
     virtual std::intmax_t integer() = 0;
 
     //! \throw wire::exception if next value not an unsigned integer.
@@ -104,8 +104,7 @@ namespace wire
     //! \throw wire::exception if next value invalid enum. \return Index in `enums`.
     virtual std::size_t enumeration(epee::span<char const* const> enums) = 0;
 
-    /*! \throw wire::exception if next value not array
-        \return Number of values to read before calling `is_array_end()`. */
+    //! \throw wire::exception if next value not array
     virtual std::size_t start_array() = 0;
 
     //! \return True if there is another element to read.
@@ -167,76 +166,58 @@ namespace wire
 
   namespace integer
   {
-    [[noreturn]] void throw_exception(std::intmax_t source, std::intmax_t min);
-    [[noreturn]] void throw_exception(std::uintmax_t source, std::uintmax_t max);
+    [[noreturn]] void throw_exception(std::intmax_t value, std::intmax_t min, std::intmax_t max);
+    [[noreturn]] void throw_exception(std::uintmax_t value, std::uintmax_t max);
 
-    template<typename Target, typename U>
-    inline Target convert_to(const U source)
+    template<typename T, typename U>
+    inline T cast_signed(const U source)
     {
-      using common = typename std::common_type<Target, U>::type;
-      static constexpr const Target target_min = std::numeric_limits<Target>::min();
-      static constexpr const Target target_max = std::numeric_limits<Target>::max();
+      using limit = std::numeric_limits<T>;
+      static_assert(
+        std::is_signed<T>::value && std::is_integral<T>::value,
+        "target must be signed integer type"
+      );
+      static_assert(
+        std::is_signed<U>::value && std::is_integral<U>::value,
+        "source must be signed integer type"
+      );
+      if (source < limit::min() || limit::max() < source)
+       throw_exception(source, limit::min(), limit::max());
+      return static_cast<T>(source);
+    }
 
-      /* After optimizations, this is:
-           * 1 check for unsigned -> unsigned (uint, uint)
-           * 2 checks for signed -> signed (int, int)
-           * 2 checks for signed -> unsigned-- (
-           * 1 check for unsigned -> signed (uint, uint)
-
-         Put `WIRE_DLOG_THROW` in cpp to reduce code/ASM duplication. Do not
-         remove first check, signed values can be implicitly converted to
-         unsigned in some checks. */
-      if (!std::numeric_limits<Target>::is_signed && source < 0)
-        throw_exception(std::intmax_t(source), std::intmax_t(0));
-      else if (common(source) < common(target_min))
-        throw_exception(std::intmax_t(source), std::intmax_t(target_min));
-      else if (common(target_max) < common(source))
-        throw_exception(std::uintmax_t(source), std::uintmax_t(target_max));
-
-      return Target(source);
+    template<typename T, typename U>
+    inline T cast_unsigned(const U source)
+    {
+      using limit = std::numeric_limits<T>;
+      static_assert(
+        std::is_unsigned<T>::value && std::is_integral<T>::value,
+        "target must be unsigned integer type"
+      );
+      static_assert(
+        std::is_unsigned<U>::value && std::is_integral<U>::value,
+        "source must be unsigned integer type"
+      );
+      if (limit::max() < source)
+        throw_exception(source, limit::max());
+      return static_cast<T>(source);
     }
   }
 
-  inline void read_bytes(reader& source, char& dest)
+  //! read all current and future signed integer types
+  template<typename T>
+  inline enable_if<std::is_signed<T>::value && std::is_integral<T>::value>
+  read_bytes(reader& source, T& dest)
   {
-    dest = integer::convert_to<char>(source.integer());
-  }
-  inline void read_bytes(reader& source, short& dest)
-  {
-    dest = integer::convert_to<short>(source.integer());
-  }
-  inline void read_bytes(reader& source, int& dest)
-  {
-    dest = integer::convert_to<int>(source.integer());
-  }
-  inline void read_bytes(reader& source, long& dest)
-  {
-    dest = integer::convert_to<long>(source.integer());
-  }
-  inline void read_bytes(reader& source, long long& dest)
-  {
-    dest = integer::convert_to<long long>(source.integer());
+    dest = integer::cast_signed<T>(source.integer());
   }
 
-  inline void read_bytes(reader& source, unsigned char& dest)
+  //! read all current and future unsigned integer types
+  template<typename T>
+  inline enable_if<std::is_unsigned<T>::value && std::is_integral<T>::value>
+  read_bytes(reader& source, T& dest)
   {
-    dest = integer::convert_to<unsigned char>(source.unsigned_integer());
-  }
-  inline void read_bytes(reader& source, unsigned short& dest)
-  {
-    dest = integer::convert_to<unsigned short>(source.unsigned_integer());
-  }
-  inline void read_bytes(reader& source, unsigned& dest)
-  {
-    dest = integer::convert_to<unsigned>(source.unsigned_integer());
-  }
-  inline void read_bytes(reader& source, unsigned long& dest)
-  {
-    dest = integer::convert_to<unsigned long>(source.unsigned_integer());
-  }
-  inline void read_bytes(reader& source, unsigned long long& dest)
-  {
-    dest = integer::convert_to<unsigned long long>(source.unsigned_integer());
+    dest = integer::cast_unsigned<T>(source.unsigned_integer());
   }
 } // wire
 
@@ -273,7 +254,7 @@ namespace wire_read
     using value_type = typename T::value_type;
     static_assert(!std::is_same<value_type, char>::value, "read array of chars as binary");
     static_assert(!std::is_same<value_type, std::uint8_t>::value, "read array of unsigned chars as binary");
-    
+
     std::size_t count = source.start_array();
 
     dest.clear();

src/wire/write.h

@@ -47,6 +47,8 @@ namespace wire
 
     virtual ~writer() noexcept;
 
+    virtual void boolean(bool) = 0;
+
     virtual void integer(int) = 0;
     virtual void integer(std::intmax_t) = 0;
 
@@ -78,6 +80,11 @@ namespace wire
 
   // leave in header, compiler can de-virtualize when final type is given
 
+  inline void write_bytes(writer& dest, const bool source)
+  {
+    dest.boolean(source);
+  }
+
   inline void write_bytes(writer& dest, const int source)
   {
     dest.integer(source);