Merge pull request #57 from AntelopeIO/bitset-pack-size

IF: pack size of bitset

libraries/chain/include/eosio/chain/finality/quorum_certificate.hpp

@@ -17,7 +17,7 @@ namespace eosio::chain {
    using bls_aggregate_signature = fc::crypto::blslib::bls_aggregate_signature;
    using bls_private_key         = fc::crypto::blslib::bls_private_key;
 
-   using vote_bitset   = boost::dynamic_bitset<uint32_t>;
+   using vote_bitset   = fc::dynamic_bitset;
    using bls_key_map_t = std::map<bls_public_key, bls_private_key>;
 
    enum class vote_status {

libraries/libfc/include/fc/bitutil.hpp

@@ -1,4 +1,6 @@
 #pragma once
+
+#include <boost/dynamic_bitset.hpp>
 #include <stdint.h>
 
 namespace fc {
@@ -25,4 +27,7 @@ inline uint32_t endian_reverse_u32( uint32_t x )
         ;
 }
 
+// Using uint8_t boost::dynamic_bitset provides a more expected raw pack/unpack format
+using dynamic_bitset = boost::dynamic_bitset<uint8_t>;
+
 } // namespace fc

libraries/libfc/include/fc/io/raw.hpp

@@ -11,15 +11,16 @@
 #include <fc/safe.hpp>
 #include <fc/static_variant.hpp>
 #include <fc/io/raw_fwd.hpp>
+#include <fc/crypto/hex.hpp>
+#include <fc/bitutil.hpp>
+
+#include <boost/multiprecision/cpp_int.hpp>
+
 #include <array>
 #include <map>
 #include <deque>
 #include <list>
 
-#include <boost/multiprecision/cpp_int.hpp>
-#include <boost/dynamic_bitset.hpp>
-#include <fc/crypto/hex.hpp>
-
 namespace fc {
     namespace raw {
 
@@ -35,8 +36,8 @@ namespace fc {
     template<typename Stream> void unpack( Stream& s,  Int<256>& n );
     template<typename Stream, typename T> void pack( Stream& s, const boost::multiprecision::number<T>& n );
     template<typename Stream, typename T> void unpack( Stream& s,  boost::multiprecision::number<T>& n );
-    template<typename Stream, typename T> void pack( Stream& s, const boost::dynamic_bitset<T>& bs );
-    template<typename Stream, typename T> void unpack( Stream& s,  boost::dynamic_bitset<T>& bs );
+    template<typename Stream> void pack( Stream& s, const fc::dynamic_bitset& bs );
+    template<typename Stream> void unpack( Stream& s,  fc::dynamic_bitset& bs );
 
     template<typename Stream, typename Arg0, typename... Args>
     inline void pack( Stream& s, const Arg0& a0, const Args&... args ) {
@@ -565,32 +566,37 @@ namespace fc {
        }
     }
 
-    template<typename Stream, typename T>
-    inline void pack( Stream& s, const boost::dynamic_bitset<T>& value ) {
+    template<typename Stream>
+    inline void pack( Stream& s, const fc::dynamic_bitset& value ) {
+      // pack the size of the bitset, not the number of blocks
       const auto num_blocks = value.num_blocks();
       FC_ASSERT( num_blocks <= MAX_NUM_ARRAY_ELEMENTS );
-      fc::raw::pack( s, unsigned_int((uint32_t)num_blocks) );
-
+      fc::raw::pack( s, unsigned_int(value.size()) );
+      constexpr size_t word_size = sizeof(fc::dynamic_bitset::block_type) * CHAR_BIT;
+      assert(num_blocks == (value.size() + word_size - 1) / word_size);
       // convert bitset to a vector of blocks
-      std::vector<T> blocks;
+      std::vector<fc::dynamic_bitset::block_type> blocks;
       blocks.resize(num_blocks);
       boost::to_block_range(value, blocks.begin());
-
       // pack the blocks
       for (const auto& b: blocks) {
-        fc::raw::pack( s, b );
+         fc::raw::pack( s, b );
       }
     }
 
-    template<typename Stream, typename T>
-    inline void unpack( Stream& s, boost::dynamic_bitset<T>& value ) {
+    template<typename Stream>
+    inline void unpack( Stream& s, fc::dynamic_bitset& value ) {
+      // the packed size is the number of bits in the set, not the number of blocks
       unsigned_int size; fc::raw::unpack( s, size );
-      FC_ASSERT( size.value <= MAX_NUM_ARRAY_ELEMENTS );
-      std::vector<T> blocks((size_t)size.value);
-      for( uint64_t i = 0; i < size.value; ++i ) {
-        fc::raw::unpack( s, blocks[i] );
+      constexpr size_t word_size = sizeof(fc::dynamic_bitset::block_type) * CHAR_BIT;
+      size_t num_blocks = (size + word_size - 1) / word_size;
+      FC_ASSERT( num_blocks <= MAX_NUM_ARRAY_ELEMENTS );
+      std::vector<fc::dynamic_bitset::block_type> blocks(num_blocks);
+      for( size_t i = 0; i < num_blocks; ++i ) {
+         fc::raw::unpack( s, blocks[i] );
       }
       value = { blocks.cbegin(), blocks.cend() };
+      value.resize(size.value);
     }
 
     template<typename Stream, typename T>

libraries/libfc/include/fc/variant_dynamic_bitset.hpp

@@ -1,31 +1,30 @@
 #pragma once
 
 #include <fc/variant.hpp>
-#include <boost/dynamic_bitset.hpp>
+#include <fc/bitutil.hpp>
+
+#include "variant_object.hpp"
 
 namespace fc
 {
-   template<typename T> void to_variant( const boost::dynamic_bitset<T>& bs, fc::variant& v ) {
+   inline void to_variant( const fc::dynamic_bitset& bs, fc::variant& v ) {
       auto num_blocks = bs.num_blocks();
-      if ( num_blocks > MAX_NUM_ARRAY_ELEMENTS ) throw std::range_error( "number of blocks of dynamic_bitset cannot be greather than MAX_NUM_ARRAY_ELEMENTS" );
+      if ( num_blocks > MAX_NUM_ARRAY_ELEMENTS )
+         throw std::range_error( "number of blocks of dynamic_bitset cannot be greather than MAX_NUM_ARRAY_ELEMENTS" );
 
-      std::vector<T> blocks(num_blocks);
+      std::vector<fc::dynamic_bitset::block_type> blocks(num_blocks);
       boost::to_block_range(bs, blocks.begin());
-
-      v = fc::variant(blocks);
+      v = fc::mutable_variant_object("size", bs.size())
+                                    ("bits", blocks);
    }
 
-   template<typename T> void from_variant( const fc::variant& v, boost::dynamic_bitset<T>& bs ) {
-      const std::vector<fc::variant>& vars = v.get_array();
-      auto num_vars = vars.size();
-      if( num_vars > MAX_NUM_ARRAY_ELEMENTS ) throw std::range_error( "number of variants for dynamic_bitset cannot be greather than MAX_NUM_ARRAY_ELEMENTS" );
-
-      std::vector<T> blocks;
-      blocks.reserve(num_vars);
-      for( const auto& var: vars ) {
-         blocks.push_back( var.as<T>() );
-      }
+   inline void from_variant( const fc::variant& v, fc::dynamic_bitset& bs ) {
+      fc::dynamic_bitset::size_type size;
+      std::vector<fc::dynamic_bitset::block_type> blocks;
 
+      from_variant(v["size"], size);
+      from_variant(v["bits"], blocks);
       bs = { blocks.cbegin(), blocks.cend() };
+      bs.resize(size);
    }
 } // namespace fc

libraries/libfc/test/io/test_raw.cpp

@@ -1,8 +1,8 @@
 #include <fc/exception/exception.hpp>
+#include <fc/bitutil.hpp>
 #include <fc/io/raw.hpp>
 
 #include <boost/test/unit_test.hpp>
-#include <boost/dynamic_bitset.hpp>
 
 using namespace fc;
 
@@ -12,14 +12,14 @@ BOOST_AUTO_TEST_SUITE(raw_test_suite)
 BOOST_AUTO_TEST_CASE(dynamic_bitset_test)
 {
    constexpr uint8_t bits = 0b00011110;
-   boost::dynamic_bitset<uint8_t> bs1(8, bits); // bit set size 8
+   fc::dynamic_bitset bs1(8, bits); // bit set size 8
 
-   char buff[4];
+   char buff[32];
    datastream<char*> ds(buff, sizeof(buff));
 
    fc::raw::pack( ds, bs1 );
 
-   boost::dynamic_bitset<uint8_t> bs2(8);
+   fc::dynamic_bitset bs2(8);
    ds.seekp(0);
    fc::raw::unpack( ds, bs2 );
 
@@ -35,4 +35,43 @@ BOOST_AUTO_TEST_CASE(dynamic_bitset_test)
    BOOST_CHECK(!bs2.test(7));
 }
 
+BOOST_AUTO_TEST_CASE(dynamic_bitset_large_test)
+{
+   fc::dynamic_bitset bs1;
+   bs1.resize(12345);
+
+   bs1.set(42);
+   bs1.set(23);
+   bs1.set(12000);
+
+   auto packed = fc::raw::pack(bs1);
+   auto unpacked = fc::raw::unpack<fc::dynamic_bitset>(packed);
+
+   BOOST_TEST(unpacked.at(42));
+   BOOST_TEST(unpacked.at(23));
+   BOOST_TEST(unpacked.at(12000));
+   unpacked.flip(42);
+   unpacked.flip(23);
+   unpacked.flip(12000);
+   BOOST_TEST(unpacked.none());
+}
+
+BOOST_AUTO_TEST_CASE(dynamic_bitset_small_test)
+{
+   fc::dynamic_bitset bs1;
+   bs1.resize(21);
+
+   bs1.set(2);
+   bs1.set(7);
+
+   auto packed = fc::raw::pack(bs1);
+   auto unpacked = fc::raw::unpack<fc::dynamic_bitset>(packed);
+
+   BOOST_TEST(unpacked.at(2));
+   BOOST_TEST(unpacked.at(7));
+   unpacked.flip(2);
+   unpacked.flip(7);
+   BOOST_TEST(unpacked.none());
+}
+
 BOOST_AUTO_TEST_SUITE_END()

libraries/libfc/test/variant/test_variant_dynamic_bitset.cpp

@@ -14,23 +14,15 @@ BOOST_AUTO_TEST_SUITE(dynamic_bitset_test_suite)
 BOOST_AUTO_TEST_CASE(dynamic_bitset_test)
 {
    constexpr uint8_t bits = 0b0000000001010100;
-   boost::dynamic_bitset<uint8_t> bs(16, bits); // 2 blocks of uint8_t
+   fc::dynamic_bitset bs(16, bits); // 2 blocks of uint8_t
 
    fc::mutable_variant_object mu;
    mu("bs", bs);
 
-   // a vector of 2 blocks
-   const variants& vars = mu["bs"].get_array();
-   BOOST_CHECK_EQUAL(vars.size(), 2u);
-
-   // blocks can be in any order
-   if (vars[0].as<uint32_t>() == bits ) {
-      BOOST_CHECK_EQUAL(vars[1].as<uint32_t>(), 0u);
-   } else if (vars[1].as<uint32_t>() == bits ) {
-      BOOST_CHECK_EQUAL(vars[0].as<uint32_t>(), 0u);
-   } else {
-      BOOST_CHECK(false);
-   }
+   fc::dynamic_bitset bs2;
+   fc::from_variant(mu["bs"], bs2);
+
+   BOOST_TEST(bs2 == bs);
 }
 
 BOOST_AUTO_TEST_SUITE_END()