lp: encode sequence number as fixed-width integer

refs #4403

Change-Id: I37daeaf3d60dbaf81a8f38ec5deb72eea6dd89c2
diff --git a/src/lp/field-decl.hpp b/src/lp/field-decl.hpp
index fa584af..26336ff 100644
--- a/src/lp/field-decl.hpp
+++ b/src/lp/field-decl.hpp
@@ -24,6 +24,7 @@
 
 #include "empty-value.hpp"
 #include "field.hpp"
+#include "sequence.hpp"
 #include "tlv.hpp"
 
 #include "../encoding/block-helpers.hpp"
@@ -33,6 +34,10 @@
 namespace ndn {
 namespace lp {
 
+/** \brief Indicate a uint64_t field shall be decoded and encoded as a non-negative integer.
+ */
+struct NonNegativeIntegerTag;
+
 template<typename TlvType, typename T>
 struct DecodeHelper
 {
@@ -54,7 +59,7 @@
   {
     if (wire.value_size() != 0) {
       BOOST_THROW_EXCEPTION(ndn::tlv::Error("NDNLP field of TLV-TYPE " + to_string(wire.type()) +
-                            " must be empty"));
+                                            " must be empty"));
     }
 
     return EmptyValue{};
@@ -62,11 +67,25 @@
 };
 
 template<typename TlvType>
+struct DecodeHelper<TlvType, NonNegativeIntegerTag>
+{
+  static uint64_t
+  decode(const Block& wire)
+  {
+    return readNonNegativeInteger(wire);
+  }
+};
+
+template<typename TlvType>
 struct DecodeHelper<TlvType, uint64_t>
 {
   static uint64_t
   decode(const Block& wire)
   {
+    // NDNLPv2 spec defines sequence number fields to be encoded as a fixed-width unsigned integer,
+    // but previous versions of ndn-cxx encode it as a NonNegativeInteger, so we decode it as such
+    // for backwards compatibility. In a future version, the decoder will be changed to accept
+    // 8-byte big endian only, to allow faster decoding.
     return readNonNegativeInteger(wire);
   }
 };
@@ -79,7 +98,7 @@
   {
     if (wire.value_size() == 0) {
       BOOST_THROW_EXCEPTION(ndn::tlv::Error("NDNLP field of TLV-TYPE " + to_string(wire.type()) +
-                            " cannot be empty"));
+                                            " cannot be empty"));
     }
 
     return std::make_pair(wire.value_begin(), wire.value_end());
@@ -111,12 +130,24 @@
 };
 
 template<typename encoding::Tag TAG, typename TlvType>
+struct EncodeHelper<TAG, TlvType, NonNegativeIntegerTag>
+{
+  static size_t
+  encode(EncodingImpl<TAG>& encoder, uint64_t value)
+  {
+    return prependNonNegativeIntegerBlock(encoder, TlvType::value, value);
+  }
+};
+
+template<typename encoding::Tag TAG, typename TlvType>
 struct EncodeHelper<TAG, TlvType, uint64_t>
 {
   static size_t
-  encode(EncodingImpl<TAG>& encoder, const uint64_t value)
+  encode(EncodingImpl<TAG>& encoder, uint64_t value)
   {
-    return prependNonNegativeIntegerBlock(encoder, TlvType::value, value);
+    uint64_t be = htobe64(value);
+    const uint8_t* buf = reinterpret_cast<const uint8_t*>(&be);
+    return encoder.prependByteArrayBlock(TlvType::value, buf, sizeof(be));
   }
 };
 
@@ -134,7 +165,16 @@
   }
 };
 
-template<typename LOCATION, typename VALUE, uint64_t TYPE, bool REPEATABLE = false>
+/** \brief Declare a field.
+ *  \tparam LOCATION a tag that indicates where the field is in an LpPacket.
+ *  \tparam VALUE type of field value.
+ *  \tparam TYPE TLV-TYPE number of the field.
+ *  \tparam REPEATABLE whether the field is repeatable.
+ *  \tparam DECODER_TAG selects a specialization of DecodeHelper.
+ *  \tparam ENCODER_TAG selects a specialization of EncodeHelper.
+ */
+template<typename LOCATION, typename VALUE, uint64_t TYPE, bool REPEATABLE = false,
+         typename DECODER_TAG = VALUE, typename ENCODER_TAG = VALUE>
 class FieldDecl
 {
 public:
@@ -143,29 +183,30 @@
   typedef std::integral_constant<uint64_t, TYPE> TlvType;
   typedef std::integral_constant<bool, REPEATABLE> IsRepeatable;
 
-  /** \brief decodes a field
-   *  \param wire a Block with top-level type \p TYPE
-   *  \return value of the field
+  /** \brief Decode a field.
+   *  \param wire an element with top-level TLV-TYPE \c TlvType::value.
+   *  \return value of the field.
+   *  \throw ndn::tlv::Error decode failure.
    */
   static ValueType
   decode(const Block& wire)
   {
     if (wire.type() != TlvType::value) {
-      BOOST_THROW_EXCEPTION(ndn::tlv::Error("Unexpected TLV type " + to_string(wire.type())));
+      BOOST_THROW_EXCEPTION(ndn::tlv::Error("Unexpected TLV-TYPE " + to_string(wire.type())));
     }
 
-    return DecodeHelper<TlvType, ValueType>::decode(wire);
+    return DecodeHelper<TlvType, DECODER_TAG>::decode(wire);
   }
 
-  /** \brief encodes a field and prepends to \p encoder its Block with top-level type \p TYPE
-   *  \param encoder Instance of the buffer encoder or buffer estimator
-   *  \param value value of the field
+  /** \brief Encode a field and prepend to \p encoder.
+   *  \param encoder a buffer encoder or estimator.
+   *  \param value value of the field.
    */
-  template<typename encoding::Tag TAG, typename T>
+  template<typename encoding::Tag TAG>
   static size_t
-  encode(EncodingImpl<TAG>& encoder, const T& value)
+  encode(EncodingImpl<TAG>& encoder, const ValueType& value)
   {
-    return EncodeHelper<TAG, TlvType, T>::encode(encoder, value);
+    return EncodeHelper<TAG, TlvType, ENCODER_TAG>::encode(encoder, value);
   }
 };
 
diff --git a/src/lp/fields.hpp b/src/lp/fields.hpp
index ce3a834..890da2b 100644
--- a/src/lp/fields.hpp
+++ b/src/lp/fields.hpp
@@ -24,7 +24,6 @@
 
 #include "field-decl.hpp"
 
-#include "sequence.hpp"
 #include "cache-policy.hpp"
 #include "nack-header.hpp"
 #include "prefix-announcement.hpp"
@@ -41,12 +40,18 @@
 
 typedef FieldDecl<field_location_tags::Header,
                   uint64_t,
-                  tlv::FragIndex> FragIndexField;
+                  tlv::FragIndex,
+                  false,
+                  NonNegativeIntegerTag,
+                  NonNegativeIntegerTag> FragIndexField;
 BOOST_CONCEPT_ASSERT((Field<FragIndexField>));
 
 typedef FieldDecl<field_location_tags::Header,
                   uint64_t,
-                  tlv::FragCount> FragCountField;
+                  tlv::FragCount,
+                  false,
+                  NonNegativeIntegerTag,
+                  NonNegativeIntegerTag> FragCountField;
 BOOST_CONCEPT_ASSERT((Field<FragCountField>));
 
 typedef FieldDecl<field_location_tags::Header,
@@ -56,7 +61,10 @@
 
 typedef FieldDecl<field_location_tags::Header,
                   uint64_t,
-                  tlv::NextHopFaceId> NextHopFaceIdField;
+                  tlv::NextHopFaceId,
+                  false,
+                  NonNegativeIntegerTag,
+                  NonNegativeIntegerTag> NextHopFaceIdField;
 BOOST_CONCEPT_ASSERT((Field<NextHopFaceIdField>));
 
 typedef FieldDecl<field_location_tags::Header,
@@ -66,12 +74,18 @@
 
 typedef FieldDecl<field_location_tags::Header,
                   uint64_t,
-                  tlv::IncomingFaceId> IncomingFaceIdField;
+                  tlv::IncomingFaceId,
+                  false,
+                  NonNegativeIntegerTag,
+                  NonNegativeIntegerTag> IncomingFaceIdField;
 BOOST_CONCEPT_ASSERT((Field<IncomingFaceIdField>));
 
 typedef FieldDecl<field_location_tags::Header,
                   uint64_t,
-                  tlv::CongestionMark> CongestionMarkField;
+                  tlv::CongestionMark,
+                  false,
+                  NonNegativeIntegerTag,
+                  NonNegativeIntegerTag> CongestionMarkField;
 BOOST_CONCEPT_ASSERT((Field<CongestionMarkField>));
 
 typedef FieldDecl<field_location_tags::Header,
@@ -94,17 +108,18 @@
                   PrefixAnnouncement,
                   tlv::PrefixAnnouncement> PrefixAnnouncementField;
 BOOST_CONCEPT_ASSERT((Field<PrefixAnnouncementField>));
-/**
- * The value of the wire encoded field is the data between the provided iterators. During
- * encoding, the data is copied from the Buffer into the wire buffer.
+
+/** \brief Declare the Fragment field.
+ *
+ *  The fragment (i.e. payload) is the bytes between two provided iterators. During encoding,
+ *  these bytes are copied from the Buffer into the LpPacket.
  */
 typedef FieldDecl<field_location_tags::Fragment,
                   std::pair<Buffer::const_iterator, Buffer::const_iterator>,
                   tlv::Fragment> FragmentField;
 BOOST_CONCEPT_ASSERT((Field<FragmentField>));
 
-/**
- * \brief set of all field declarations
+/** \brief Set of all field declarations.
  */
 typedef boost::mpl::set<
   FragmentField,
diff --git a/tests/unit-tests/lp/packet.t.cpp b/tests/unit-tests/lp/packet.t.cpp
index 5dafc20..5f9b2d2 100644
--- a/tests/unit-tests/lp/packet.t.cpp
+++ b/tests/unit-tests/lp/packet.t.cpp
@@ -66,9 +66,9 @@
 BOOST_AUTO_TEST_CASE(EncodeFragment)
 {
   static const uint8_t expectedBlock[] = {
-    0x64, 0x08, // LpPacket
-          0x51, 0x02, // Sequence
-                0x03, 0xe8,
+    0x64, 0x0e, // LpPacket
+          0x51, 0x08, // Sequence
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0xe8,
           0x50, 0x02, // Fragment
                 0x03, 0xe8,
   };
@@ -78,10 +78,9 @@
   buf[1] = 0xe8;
 
   Packet packet;
-  BOOST_CHECK_NO_THROW(packet.add<FragmentField>(std::make_pair(buf.begin(), buf.end())));
-  BOOST_CHECK_NO_THROW(packet.add<SequenceField>(1000));
-  Block wire;
-  BOOST_CHECK_NO_THROW(wire = packet.wireEncode());
+  packet.add<FragmentField>(std::make_pair(buf.begin(), buf.end()));
+  packet.add<SequenceField>(1000);
+  Block wire = packet.wireEncode();
   BOOST_CHECK_EQUAL_COLLECTIONS(expectedBlock, expectedBlock + sizeof(expectedBlock),
                                 wire.begin(), wire.end());
 }
@@ -129,17 +128,17 @@
 BOOST_AUTO_TEST_CASE(EncodeSortOrder)
 {
   static const uint8_t expectedBlock[] = {
-    0x64, 0x19, // LpPacket
+    0x64, 0x2e, // LpPacket
           0x52, 0x01, // FragIndex
                 0x00,
           0x53, 0x01, // FragCount
                 0x01,
-          0xfd, 0x03, 0x44, 0x01, // Ack
-                0x02,
-          0xfd, 0x03, 0x44, 0x01, // Ack
-                0x04,
-          0xfd, 0x03, 0x44, 0x01, // Ack
-                0x03,
+          0xfd, 0x03, 0x44, 0x08, // Ack
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+          0xfd, 0x03, 0x44, 0x08, // Ack
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04,
+          0xfd, 0x03, 0x44, 0x08, // Ack
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
           0x50, 0x02, // Fragment
                 0x03, 0xe8,
   };